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

Created #positive_definite? #561

Merged
merged 4 commits into from Mar 22, 2017
Merged

Conversation

shardulparab97
Copy link

@shardulparab97 shardulparab97 commented Jan 6, 2017

Pull request in response to #411

@shardulparab97
Copy link
Author

Can someone also please point out as to why am I receiving a build error with this code.
Thanks.

@agisga
Copy link

agisga commented Jan 6, 2017

Can someone also please point out as to why am I receiving a build error with this code.

From the travis-ci log, it looks like the variable dtype in your spec is undefined:

/home/travis/build/SciRuby/nmatrix/spec/math_spec.rb:1064:in block in <top (required)>': undefined local variable or method dtype' for #<Class:0x00000002faeb30> (NameError)

@shardulparab97
Copy link
Author

Thank you @agisga

raise(ShapeError, "positive definite calculated only for square matrices") unless self.dim == 2 && self.shape[0] == self.shape[1]
ans=true
cond=0
while cond!=self.cols
Copy link
Member

Choose a reason for hiding this comment

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

Please adjust if possible to NMatrix coding style.

@@ -1060,4 +1060,13 @@
end
end
end

context "#positive_definite? for #{dtype}" do
Copy link
Member

Choose a reason for hiding this comment

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

This is not working because you've added it outside the dtype context block. Please have a closer look at the file structure. You can also test your specs on your own machine by doing bundle exec rake spec.

@translunar translunar changed the title Created #positive_deficient? Created #positive_definite? Jan 10, 2017
@shardulparab97
Copy link
Author

shardulparab97 commented Jan 14, 2017

@MohawkJohn I have made changes adhering to the NMatrix coding style, following Github's Ruby Styleguide.
Please let me know if I have missed out on something.
Thank you.

@translunar
Copy link
Member

@v0dro Here's another one. This one needs a squash and merge (once you're reviewed it also, of course).

@gtamba
Copy link

gtamba commented Feb 20, 2017

@shardulparab97 : Can I recommend that you open a new issue since #411 has a few other functions listed in its agenda as well? (Since there is also a stalled pull request #461 that was attempting to solve #411 completely this would help keep things a bit saner)

Alternatively you could just refer to #461 if you wish to complete the entire function set yourself but this seems perfectly fine too.

@v0dro : I'm not sure why #461 was closed by the author because it was as good as complete, with a few changes pending, those were a bunch of simple yet handy functions.

@translunar translunar merged commit a98d7fd into SciRuby:master Mar 22, 2017
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.

None yet

4 participants