-
Notifications
You must be signed in to change notification settings - Fork 63
Bpw/sturm lcpp #235
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
Bpw/sturm lcpp #235
Conversation
|
I believe issue #239 is what's causing the last of the fails |
jbrzensk
left a comment
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.
Thanks @bewagner1 for this. I added some notes to the first example, which can be applied to the others.
The docs have some small nitpick items. The code as well. Basically, if something is constant, declare it as such.
For the armadillo vectors and matrices, call them as arma::vec or arma::mat so the reader knows, even though mole uses the armadillo namespace.
I checked the solutions, they seem on par with the MATLAB. You need to add some text about what the output is. See the comment in the code.
Thanks Ben!!
Requested changes were addressed
What type of PR is this? (check all applicable)
Description
This PR contains the current Sturm-Liouville Matlab examples now implemented in C++ along with updated documentation for all Sturm-Liouville examples.
Related Issues & Documents
QA Instructions, Screenshots, Recordings
My machine is an M1 MacBook Pro. The norms of the difference between the MOLE solution and exact solution are the same as in the Matlab implementation.
Added/updated tests?
_We encourage you to test all code included with MOLE, including examples.
Read Contributing Guide and Code of Conduct
[optional] Are there any post deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?