maxpkatz
commented
6 years ago Parameter changes I think are unambiguously good:
- castro.use_retry = 1
Retries are a great way to avoid CFL violations that result in the simulation dying. Most newer users do not even realize this is a possible failure mode until it hits them deep into a simulation and it causes significant disruption. One small caveat here is that retries are a tool that have required a bit of tinkering in the past to get right. So I cannot guarantee that future changes to the scheme will not change code results in cases where the retries are hit.
- castro.hard_cfl_limit = 0
This should be done in conjunction with the retry default, or else we'll never even get to the retry because we crash immediately upon detecting a CFL violation.
- castro.source_term_predictor = 1
Doing a lagged prediction of the source terms in the hydro should always be more accurate.
- castro.use_custom_knapsack_weights = 1
This is the sort of performance option that the user should never have objection to -- in general, we should do performance improvements for the user unless we think there is any reason they would not want this.
Changes I think are very good but have at least some reason for caution:
- castro.limit_fluxes_on_small_dens = 1
It's always nice to avoid density resets; density resets are quite bad and unpredictable. The reason for caution here is that the flux limiter can also (slightly) modify the flow in smooth regions that don't quite get to a reset.
Changes I think are good but are somewhat a matter of personal choice:
- castro.state_interp_order = 0
We have seen that the linear state interpolation can have problems keeping sum(X) = 1 during a fillpatch. It is not good for users to crash for a reason like this during an AMR run.
- ~castro.update_state_between_sources = 0~
~The default now is that after every source term is calculated at the new time, we update the state immediately before calculating the next source term. This results in an inconsistency where different source terms see a different new-time state, and also involves an EOS call before every extra source term.~
zingale
The default behavior of Castro should be to use the values of the runtime parameters that give the best accuracy or performance. Some default values have not been updated in a long time, despite code improvements. We should propose parameter changes in this issue.