-
Notifications
You must be signed in to change notification settings - Fork 192
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 bug in rootfind for enthalpy parametrization #4536
Fix bug in rootfind for enthalpy parametrization #4536
Conversation
@@ -14,7 +14,7 @@ | |||
#include "PointwiseFunctions/Hydro/EquationsOfState/Factory.hpp" | |||
#include "PointwiseFunctions/Hydro/SpecificEnthalpy.hpp" | |||
#include "Utilities/ErrorHandling/Error.hpp" | |||
#include "Utilities/MakeWithValue.hpp" | |||
#include "Utilities/MakeWithValue.hpp |
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.
missing quotation
82e8b97
to
2df1957
Compare
// Guarantee that the root find has the correct bracket by explicitly | ||
// nonsensical there, so if the correct braket |
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.
Please fix this sentence
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.
We need this merged ASAP because we are doing large simulations on Wheeler over the break. I think the sentence, while not great, is clear enough that it would be better for our science purposes to not delay this PR. Given some folks (including myself) are already on vacation and only doing this that are needed to do simulations over the break, I'd like to not block this PR on the sentence...
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.
sure, please fix later then
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.
Yep, Thanks!
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.
Just FYI, as a reminder: #4537
Proposed changes
Change the limits for rootfind in computing density as a function of specific enthalpy
Upgrade instructions
Code review checklist
make doc
to generate the documentation locally intoBUILD_DIR/docs/html
.Then open
index.html
.code review guide.
bugfix
ornew feature
if appropriate.Further comments