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

Add Sum Method by keithdoggett · Pull Request #51 · SciRuby/numruby · GitHub

Add Sum Method - #51

Open
keithdoggett wants to merge 1 commit into
SciRuby:masterfrom
keithdoggett:sum-method
Open

Add Sum Method#51
keithdoggett wants to merge 1 commit into
SciRuby:masterfrom
keithdoggett:sum-method

Conversation

keithdoggett commented May 25, 2020
edited
Loading

Copy link
Copy Markdown

Added sum instance method in the elementwise module and associated tests in elementwise_test.

keithdoggett changed the title added sum method for all data types Add Sum Method May 25, 2020

developerfab left a comment

Copy link
Copy Markdown

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

I have left some comments. I have a question, what is the gem that use this project to do the tests?

Comment thread ext/elementwise.c
{
complex double* input_elements = (complex double*)input->elements;
complex double sum = 0 + 0*I;
for(size_t index = 0; index < input->count; index++){

Copy link
Copy Markdown

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

I have seen that you have this cycle in each case of this condition, I think that you could move this in a function, WDYT?. Also I think that you could create a function that calcule this operation and in each case "cast" the response, but maybe this could be weird 🤔

Comment thread ext/elementwise.c
Data_Get_Struct(self, nmatrix, input);

VALUE result;

Copy link
Copy Markdown

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

I think that this line isn't necessary

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
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

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL