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 normalization efforts for true SDC #1583

Closed

Conversation

zingale
Copy link
Member

@zingale zingale commented Mar 5, 2021

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

@zingale
Copy link
Member Author

zingale commented Mar 7, 2021

here are the tests: http://groot.astro.sunysb.edu/Castro/test-suite/gfortran/2021-03-07-003/index.html

I think they look good. In particular, this fixes some crashes with the detonation that arose when we added the abort for the mass fraction non-conservation. I think that this is much more robust now.

@zingale zingale marked this pull request as ready for review March 7, 2021 17:51
@maxpkatz
Copy link
Member

maxpkatz commented Mar 7, 2021

If I understand this correctly, it's basically doing what the code was doing before we merged #1379, except it's being handled closer to where the negative mass fraction is being generated instead of letting the generic clean_state do it for you. The concern I have is that the point of #1379 is that if you got to the point where we would be aborting for negative X, it's an indication that your algorithm actually did something really bad, and papering it over with a floor/normalization is not the right thing to do. If VODE is generating a negative X (for example), the point of #1379 is that we should be fixing VODE not to do that, instead of just shifting where the hiding of the negative X gets inserted.

@zingale
Copy link
Member Author

zingale commented Mar 7, 2021

I mostly agree. In the Strang stuff, we do a normalization at the end of the solve as well, so this is consistent with that.

But I think that what we also need is a TRUE_SDC code path in VODE that does the valid_update check on the partial densities. I think that together with this will solve a lot of the issues.

@maxpkatz
Copy link
Member

maxpkatz commented Mar 7, 2021

Yes, I would be OK with this if we also did the proposed VODE fix.

@zingale zingale added the sdc label Sep 20, 2023
@zingale
Copy link
Member Author

zingale commented Oct 5, 2023

I think that most of these are now in #2586

@zingale zingale closed this Oct 5, 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