-
Notifications
You must be signed in to change notification settings - Fork 8
[Draft] Fix DO_TITER option #186
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
base: develop
Are you sure you want to change the base?
Conversation
I probably won’t get to this properly until April 28, but can you tell me more about why you need this feature? If there’s a need for it, we should add a test to the CI to ensure it stays in a working state. I personally only used it for a one-off experiment many years ago. |
We're currently running some experiments including a pure H/He envelope. In those cases the ionization stage seems to jump in between timesteps (in case of H from mostly neutral to fully ionizes), causing issues in e.g. the electron temperature etc. @ssim suggested that using multiple iterations per timestep may help to converge the ionization stage (given that artis lacks the infrastructure to limit the maximum change in ionization). Assuming that this solves our issues, I would have a more long-term need for this feature as several of my models would involve such envelopes. |
Ok! If this is going to be useful, I suggest we add another artisoptions.h function, something like: constexpr int NUM_TIMESTEP_ITERATIONS(int timestep) { return (timestep <= -1) ? 3 : 1; } Then this can be enabled for one of the CI tests (maybe in classicmode_1d) and we'll store the checksums. Hopefully you're using classic options? The timestep iteration stuff isn't implemented for the binned radiation field or the detailed bound-free estimators that we use for NLTE calculations. |
Ah, this explains why it runs in my test setup (classic), but not in my production setup, which runs on NLTE settings (or at least a binned radiation field) since we're mostly interested in nebular spectra. Is this something that could be implemented rather easily or would this require some major code changes? I'm willing to do most of the heavy lifting here, at least as far as my expertise goes. |
Yes, I think you could do it with a bit of work. If you happen to come up with a neat way to do this (i.e. not just duplicating all of the arrays with a ‘_save’ suffix) that would be even better, but even the current way extended to those extra features would be fine to merge. |
In the current
develop
branch, enabling theDO_TITER
does not seem to work. Superficially, this is a compile issue, but I'm not sure if there are some deeper issues.So far I've been able to fix the compile issues and get it to run with 3 iterations for timesteps after the first timestep (i.e. setting
globals::n_titer = (globals::timestep > 1) ? 3 : 1;
), but my test set 8000 up crashes later for some other reason I haven't been able to figure out. Somebody more knowledgable than me may need to run some verification tests to make sure this works as intended.