FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Convolutions in 1d in rust by eengamer2007 · Pull Request #1007 · algorithm-archivists/algorithm-archive · GitHub

/ algorithm-archive Public

Convolutions in 1d in rust - #1007

Open
eengamer2007 wants to merge 17 commits into
algorithm-archivists:mainfrom
eengamer2007:convolutions_in_1d_in_rust
ConversationCommits (17)ChecksFiles changed
Open

Convolutions in 1d in rust#1007
eengamer2007 wants to merge 17 commits into
algorithm-archivists:mainfrom
eengamer2007:convolutions_in_1d_in_rust

Conversation

eengamer2007 commented Nov 19, 2022

Copy link
Copy Markdown

implemented the convolutions 1d in rust

Amaras added Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) lang: rust Rust programming language labels Nov 20, 2022
Amaras reviewed Nov 20, 2022
View reviewed changes

Amaras left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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)

Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
Comment thread contents/convolutions/1d/code/rust/1d_convolution.rs Outdated
eengamer2007 and others added 7 commits November 21, 2022 19:16
replace for-loop with iterator
e8573e2 
Co-authored-by: Sammy Plat <sammy@sammyplat.fr>
replaced reference to vec with a slice
067c929 
Co-authored-by: Sammy Plat <sammy@sammyplat.fr>
replace write! with writeln!
fc70dc0 
Not sure why I didn't do it in the first place.

Co-authored-by: Sammy Plat <sammy@sammyplat.fr>
replace write! with writeln!
9316119 
Co-authored-by: Sammy Plat <sammy@sammyplat.fr>
replace write! with writeln!
9054525 
Co-authored-by: Sammy Plat <sammy@sammyplat.fr>
use iterator instead of indexing
7fb386f 
Co-authored-by: Sammy Plat <sammy@sammyplat.fr>
add forgotten ;
6c52d0a 
Co-authored-by: Sammy Plat <sammy@sammyplat.fr>

eengamer2007 commented Nov 22, 2022 •
edited
Loading

Copy link
Copy Markdown
Author

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?

leios commented Nov 22, 2022

Copy link
Copy Markdown
Member

Yeah, add that to this pr, if you can

Amaras commented Jan 13, 2023

Copy link
Copy Markdown
Member

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.
I don't see a reason to refuse your code in the AAA now, so please make sure the lines you include your code at are indeed the correct ones before I merge them.

eengamer2007 commented Feb 6, 2023

Copy link
Copy Markdown
Author

sorry for the late fix, school was keeping me busy.

eengamer2007 requested a review from Amaras February 10, 2023 09:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Amaras Awaiting requested review from Amaras

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) lang: rust Rust programming language

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL