-
Notifications
You must be signed in to change notification settings - Fork 98
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
some changes to the SDC Newton solver to make it more robust #2586
some changes to the SDC Newton solver to make it more robust #2586
Conversation
we now reject a solve if the mass fractions are too far out of [0, 1] also do the total energy update conservatively
test diffs are small: |
for (int n = 0; n < NumSpec; ++n) { | ||
if (U_new[UFS+n] < -newton::species_failure_tolerance * U_new[URHO] || | ||
U_new[UFS+n] > (1.0_rt + newton::species_failure_tolerance) * U_new[URHO]) { | ||
ierr = newton::BAD_MASS_FRACTIONS; |
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.
Does it makes sense to keep doing the sub-stepping if we hit this error case?
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.
oh, you mean I should do a break
here?
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.
Something like that, yes
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.
break added
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.
Well I'm not as worried about a break inside the species loop, I mean a break after the species loop before we finish up anything else we do below this
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.
ah, okay
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.
I just moved it to before the previous break. Thanks. I missed that.
|
we now reject a solve if the mass fractions are too far out of [0, 1] also do the total energy update conservatively
PR summary
PR motivation
PR checklist
CHANGES
file has been updated, if appropriate