-
Notifications
You must be signed in to change notification settings - Fork 20
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
Initial guess for temperature #310
Comments
An alternate solution might be something along the lines of #534. The advantage here is that it is very easy to follow and understand, and currently works to solve the issue even through restart. |
The main unsolved issue IMO is how we get the lazy eval infrastructure to enumerate this as an input. #534 does nothing in that regard. See inducer/arraycontext#101 for infrastructure discussion on that. I also really do not like the blatantly stateful nature of that proposed change. |
I sensed that #534 would run into a lot of resistance. However, it simply illustrates what we are trying to do, and is adjacent to what application folks are thinking about when having a hard time understanding why using the temperature from the previous step needs to be so complicated. In your current plan, the thing that we want to work (plz correct if I'm off-base here) I think is this:
where There may be enough nuance here (e.g. one state has it; one state doesn't, new state needs to store as a different field, etc) to preclude handling this in-general down in the array container guts. The capability developer will need some detailed control over how the non-participatory fields are propagated forward through operators. |
Think about it from this angle: In order to "compile" an array expression, we need to be able to enumerate all inputs to it. Everything that's not explicitly an argument (i.e. an input) to the function being compiled is treated as "constant data", i.e. it is assumed to not change between function evaluations. That's a fair assumption for things like differentiation matrices or Jacobians, but not a fair assumption for this temperature initial guess. An easy solution is to simply pass the temperature initial guess as an explicit additional input to the RHS, but then this changes lots of interfaces (time steppers etc.) that shouldn't need to change. Hence my thought to "ship this along" in the solution container. |
Thanks! This explanation and @kaushikcfd's similar one are very helpful.
Please have a fresh look at #534 when you have a chance. After @kaushikcfd and I discussed at length, we are trying a reference state approach where we are just passing the |
This issue is intended to continue a conversation about how to handle the initial temperature guess that we pass into
pyro.get_temperature
in Pyrometheus-based EOS in MIRGE-Com.Currently, if we do not have a reasonable guess,
get_temperature
takes many iterations, and can sometimes apparently get the wrong answer.Current approach: just pass a constant default (e.g. 300K) (expensive, potentially wrong)
Suggested: pass the temperature from previous step (ideal, but tricky, requires restarting temperature else we get a different temperature upon restart!)
Suggested: pass a deterministic guess that we compute based on current state (promising, but tricky to craft the routine for a deterministic guess)
The ultimate plan is to use the temperature from the previous step. Here is the current plan to do that (as I understand it) advised by @inducer:
CV
objectdeserialize_container
(and maybeserialize_container
, too) forCV
which will reconstruct a newCV
out of a previously serializedCV
.CV
?@inducer, @ecisneros8, @anderson2981
The text was updated successfully, but these errors were encountered: