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

VODE steps fail if the state is not valid #350

Merged
merged 3 commits into from
Jul 21, 2020

Conversation

maxpkatz
Copy link
Member

Commit 9d3ddb1 added intentional failure modes to the VODE integrator to prevent VODE from giving us a state where the integrator believed it was successful but the state was not physically valid. Specifically, we checked on T < 0 and X < -0.01 or X > 1.01. Although this gave us the opportunity to then retry the burn, retries are expensive and we'd like to avoid them if possible (#106). One way we can significantly mitigate this is by preventing these failure modes from occurring at the end of each step rather than only at the end of the burn. We can then rely on the integrator's built-in step retry mechanism (which decreases dt or changes the integration order).

I chose to use the same failure modes as the one at the end of the burn. Additionally, I added a failure mode if any X changes by more than a factor of 2. The abundance never changes that much in a single step in a typical burn, but I have witnessed some extreme cases where the abundance floors in a single integration step and VODE doesn't reject the update. (I am not sure why.)

// in any integration step.
const Real vode_increase_change_factor = 2.0_rt;
const Real vode_decrease_change_factor = 0.5_rt;
const Real vode_change_factor_X_threshold = 1.0e-6_rt;
Copy link
Member

Choose a reason for hiding this comment

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

I think vode_change_factor_X_threshold should be max(1.e-6, atol_spec)

Copy link
Member Author

@maxpkatz maxpkatz Jul 20, 2020

Choose a reason for hiding this comment

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

Maybe I should make it just atol_spec?

Copy link
Member

Choose a reason for hiding this comment

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

that's fine too

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@zingale
Copy link
Member

zingale commented Jul 20, 2020

can you update the CHANGES.md?

@zingale
Copy link
Member

zingale commented Jul 20, 2020

all tests pass: http://groot.astro.sunysb.edu/Microphysics/test-suite/gfortran/2020-07-20-001/index.html
so it seems we don't have any coverage for this change in the suite?

@maxpkatz
Copy link
Member Author

all tests pass: http://groot.astro.sunysb.edu/Microphysics/test-suite/gfortran/2020-07-20-001/index.html
so it seems we don't have any coverage for this change in the suite?

As far as I can tell, this doesn't affect normal burns, it just affects burns that would have failed anyway with (say) X > 1. By construction, we didn't have this in the suite, because all our burns in the suite pass :-)

@zingale
Copy link
Member

zingale commented Jul 21, 2020

okay, I'm happy with this one the CHANGES.md is updated

@maxpkatz
Copy link
Member Author

can you update the CHANGES.md?

Done

@zingale zingale merged commit 520b7ca into AMReX-Astro:development Jul 21, 2020
@maxpkatz maxpkatz deleted the vode_hardening branch July 21, 2020 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants