-
-
Notifications
You must be signed in to change notification settings - Fork 356
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
Convolutions in 1d in rust #1007
base: main
Are you sure you want to change the base?
Convolutions in 1d in rust #1007
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thank you for your interest in the AAA.
I've run clippy (with -W clippy::pedantic
as well) on your code and put all its suggestions here, as well as some suggestions and questions of mine.
I also don't like the fact that you cast a lot between usize
and isize
in convolve_linear
and convolve_cyclic
, but I'm not sure how you could do otherwise without increasing complexity (and it makes pedantic clippy complain a lot)
Co-authored-by: Sammy Plat <[email protected]>
Co-authored-by: Sammy Plat <[email protected]>
Not sure why I didn't do it in the first place. Co-authored-by: Sammy Plat <[email protected]>
Co-authored-by: Sammy Plat <[email protected]>
Co-authored-by: Sammy Plat <[email protected]>
Co-authored-by: Sammy Plat <[email protected]>
Co-authored-by: Sammy Plat <[email protected]>
i saw somewhere that I should add my name to the contributers list since this is my first contribution. Should that be in this pull request? |
Yeah, add that to this pr, if you can |
…-> isize to Fn(isize, usize) -> usize
Oof, sorry for the delay in the response. So, the main problem with your PR currently is that the line numbers don't track correctly. |
sorry for the late fix, school was keeping me busy. |
implemented the convolutions 1d in rust