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

Fix gradient function and add test #5321

Merged
merged 1 commit into from
Jan 7, 2014
Merged

Fix gradient function and add test #5321

merged 1 commit into from
Jan 7, 2014

Conversation

andreasnoack
Copy link
Member

Fix #5320

@andreasnoack
Copy link
Member Author

This one should also work for empty vectors except if they of type None. I think it is unlikely that someone will call the function with a Vector{None}.

@gitfoxi
Copy link
Contributor

gitfoxi commented Jan 7, 2014

So, before there was not one single test for gradient. Just glancing at
the code if n > 0 short circuits the rest of the function which is broken
anyway. I'm not suggesting to go all corporate, but if I were the product
manager for JuliaLang I would freeze all new features until there was some
minimal test coverage for every existing one and then require coverage for
anything going to master going forward.

On Tue, Jan 7, 2014 at 7:35 AM, Andreas Noack Jensen <
[email protected]> wrote:

This one should also work for empty vectors except if they of type None.
I think it is unlikely that someone will call the function with a
Vector{None}.


Reply to this email directly or view it on GitHubhttps://github.com//pull/5321#issuecomment-31747362
.

Michael

@JeffBezanson
Copy link
Member

I'm pretty sure we already do require test coverage for all new features. The gradient function is probably very old.

@andreasnoack
Copy link
Member Author

Git suggests that it dates back from 2012-04-21 so I think it is one of the older functions in linalg.

JeffBezanson added a commit that referenced this pull request Jan 7, 2014
Fix gradient function and add test
@JeffBezanson JeffBezanson merged commit c59c2e7 into master Jan 7, 2014
@andreasnoack andreasnoack deleted the anj/fixgrad branch January 7, 2014 21:06
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.

The gradient function returns incorrect results
3 participants