Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trait definition tweaks for version 2.0 #1

Open
Skeletonxf opened this issue Mar 20, 2021 · 0 comments
Open

Trait definition tweaks for version 2.0 #1

Skeletonxf opened this issue Mar 20, 2021 · 0 comments

Comments

@Skeletonxf
Copy link
Owner

Skeletonxf commented Mar 20, 2021

Version 2.0 of Easy ML is not planned any time soon likely to be some version not long after 1.10, some minor breaking changes to trait definitions are planned for version 2.0 in order to make the library more straightforward and eliminate some rough edges. These should help bring the Matrix view code up to parity with the Tensor view code that benefited from some of the oversights in the Matrix views being already known and not repeated.

The Real trait should inherit from the Numeric trait

Usability shouldn't be much affected by having to specify both traits in all trait bounds on 1.x versions, and should upgrade to 2.0 without issue.

MatrixRef should inherit from NoInteriorMutability

What first seemed like a sensible balance for MatrixRef's built in interior mutability guarantees became less useful the moment I started working with &mut versions of methods and iterators. Similarly, most of the numerical operations for MatrixViews all require NoInteriorMutability since they need to know that the size of the matrix is the same as when they check size() versus when they iterate through the elements.
Instead of deleting the NoInteriorMutability trait that has been added in 1.8 in a future 2.0 release, I think it will be an easier change if MatrixRef is made to require it. This should avoid breaking any code which currently specifies both trait bounds, and it enforces the change in contract on MatrixRef in the type system, so unsafe code written for 1.7+ versions which would not comply with NoInteriorMutability will not silently become wrong in an upgrade.
Usability is more affected by having to carry NoInteriorMutability around in 1.8+ versions since it makes MatrixViews that use trait objects require some boilerplate with a base trait that combines MatrixRef/MatrixMut in order to use many methods. However, users would have to create a base trait for this anyway if they need additional behaviour like Debug, and the boilerplate can be provided through a doc example so users don't need to come up with it themselves. Again, existing code which specifies both traits in trait bounds should upgrade to 2.0 without issue.

MatrixRef and MatrixMut traits should feature blanket impls for &S and &mut S where S already implements MatrixRef/MatrixMut. Similarly NoInteriorMutability should get blanket impls even as it gets phased out.

Unfortunately this is breaking, since anyone can already write T, &T and &mut T implementations for a custom type (and they are likely to want to for ease of calling generic methods), and the blanket impls would then add a second definition. Blanket impls can still be written on custom wrapper types as a workaround until a version 2.0.

Numeric should inherit from Debug

Essentially all number types will implement Debug already, but without this as part of the trait it can be awkward to print any kind of debug logs, panic with any kind of message, or return an error type with a useful message when working with generic number types.

@Skeletonxf Skeletonxf changed the title The Real trait should inherit from the Numeric trait Trait definition tweaks for version 2.0 Dec 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant