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

Save new Reactions_Type data after a simplified SDC burn #929

Merged
merged 1 commit into from
May 7, 2020

Conversation

dwillcox
Copy link
Member

@dwillcox dwillcox commented May 7, 2020

@jmsexton03 has reported that for Simplified SDC in the flame problem, enuc in the plotfiles is identically zero. This is not the case for Strang and the reason appears to be a bug.

This PR will ensure new Reactions_Type data is saved after the burn properly so enuc will be saved into the plotfiles correctly, corresponding to the value at the last SDC iteration. It should not change the estimated timestep because we aren't currently using Reactions_Type to do so.

To elaborate:

Currently in Simplified SDC, Castro::react_state() sets the new Reactions_Type data (R_new) in ca_react_state_simplified_sdc.

There we update Unew using the results of the burn, however, right before returning from react_state() we overwrite the burn results in R_new with the old values in Reactions_Type in R_old in the MultiFab::Copy().

This was done in 3460254 because at that time the burning timestep estimator required the Reactions_Type data to be defined. (Although even then it seemingly makes more sense to save R_new back into R_old after the burn instead of vice versa?).

As of 37e048c the timestep checking logic no longer requires the Reactions_Type data in the deprecated ca_check_timestep. We are just calling the actual_rhs to get an instantaneous value for d(Enuc)/dt. Although in Castro::estTimeStep() we are passing R_new into ca_estdt_burning, it remains unused there.

@maxpkatz
Copy link
Member

maxpkatz commented May 7, 2020

Although even then it seemingly makes more sense to save R_new back into R_old after the burn instead of vice versa?

Your confusion there is because we used to store the simplified SDC burn in R_old for reasons I don't remember. That was changed relatively recently (af8b9cf ), but I missed this bit in react_state when I did that.

@zingale zingale merged commit 26543e6 into AMReX-Astro:development May 7, 2020
@dwillcox dwillcox deleted the reactions_type_fix branch May 7, 2020 23:55
zingale pushed a commit that referenced this pull request May 8, 2020
#930)

This PR saves n_rhs + 2*n_jac from the integrator into the Reactions_Type variable weights for simplified SDC as is currently done for Strang.

To see this in the plotfiles requires first merging #929
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.

3 participants