-
Notifications
You must be signed in to change notification settings - Fork 45
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
suggestion - add num::Zero or Default constraint to MulAcc #285
Comments
I think we use a combination of |
ok fair enough. would it be overkill to do trait MulAcc : Zero+Default{} ? perhaps various parts of the code only require one or the other and you prefer to keep it seperable (eg incase of a future split ). |
Either we have to require I think places we have |
imagine
trait MulAcc : num_traits::Zero{..}
- the types satisfying this (for dot product output and matrix multiply output) usually want to be zero-able to initialize them. Often we see bounds like thisN: 'a + Clone + crate::MulAcc + num_traits::Zero,
(infact would it be reasonable to demand that an accumulator type is Cloneable?)many of the helper functions throughout need to qualify this extra Zero constraint because they do this intialization.
As an alternative for further generality you might want to consider "Default" rather than "Zero"- this would further open up options in fitting other calculations to the pattern of matrix-multiply. Default kind of means "empty", e.g. Vec::default()=vec![]. an "empty" accumulator is zero. it's also a core trait so more likely a user has already got it implemented.
i've verified that the stdlib provides i32::default()==0 f32::default()==0, f64::default()==0 .. put numeric fields in a struct with #[derive(Default)] and you get them cleared.
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=37acf55a89cde8c5b066736c2f3d5142
I haven't put this into my PR for MulAcc<A,B> as I dont want to flood too many changes at once (especially the idea of default instead of zero..)
The text was updated successfully, but these errors were encountered: