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

some changes to the SDC Newton solver to make it more robust #2586

Merged
merged 16 commits into from
Oct 12, 2023

Conversation

zingale
Copy link
Member

@zingale zingale commented Sep 25, 2023

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

  • test suite needs to be run on this PR
  • this PR will change answers in the test suite to more than roundoff level
  • all newly-added functions have docstrings as per the coding conventions
  • the CHANGES file has been updated, if appropriate
  • if appropriate, this change is described in the docs

we now reject a solve if the mass fractions are too far out of [0, 1]
also do the total energy update conservatively
@zingale zingale added the sdc label Oct 3, 2023
@zingale zingale changed the title [WIP] some changes to the SDC Newton solver to make it more robust some changes to the SDC Newton solver to make it more robust Oct 9, 2023
@zingale
Copy link
Member Author

zingale commented Oct 9, 2023

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;
Copy link
Member

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?

Copy link
Member Author

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?

Copy link
Member

Choose a reason for hiding this comment

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

Something like that, yes

Copy link
Member Author

Choose a reason for hiding this comment

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

break added

Copy link
Member

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

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, okay

Copy link
Member Author

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.

@zingale
Copy link
Member Author

zingale commented Oct 10, 2023

bubble_convergence converges as expected for SDC-4:

 $\rho$                           3.585788e+15   3.265       3.729242e+14   3.727       2.816993e+13
 $\rho u$                         1.118655e+24   3.796       8.055982e+22   3.961       5.174599e+21
 $\rho v$                         1.312309e+24   3.544       1.124968e+23   3.852       7.792157e+21
 $\rho E$                          3.69012e+32   2.949       4.780079e+31   3.669        3.75798e+30
 $\rho e$                         3.689677e+32   2.949       4.779642e+31   3.669       3.757779e+30
 $T$                              1.437985e+18   3.508       1.264001e+17   3.831       8.884044e+15
 $\rho X(\isotm{He}{4})$          3.584552e+15   3.268       3.722151e+14   3.725       2.814812e+13
 $\rho X(\isotm{C}{12})$          1.519908e+13   2.544       2.605549e+12   3.797       1.874236e+11
 $\rho X(\isotm{O}{16})$              35854370   3.265            3729525   3.727           281671.7
 $\rho X(\isotm{Fe}{56})$             35856870   3.265            3729094   3.727           281699.3

@zingale zingale merged commit f8446cd into AMReX-Astro:development Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants