-
Notifications
You must be signed in to change notification settings - Fork 69
Unsteady heat support #2388
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
Unsteady heat support #2388
Conversation
8c094ee to
cdd748a
Compare
dbochkov-flexcompute
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a couple of big picture comments:
-
Not sure about having output frequency as a parameter in
UnsteadySpec. I think it could be more useful if sampling frequency can be defined for each monitor separately, similar to fdtd time monitors. Say, if I want to have just few snapshots in time for a big monitor, but then also record temperature for each step at a given point. -
For initial condition, I imagine at some point we might need to extend it to non-uniform distribution. While I would say this is out of scope for this PR, I think it would be worth ironing out how that would look like. I see two options
initial_temperature: Union[pd.PositiveFloat, SpatialDataArray, UnstructuredGridDataset]initial_temperature: Union[UniformDistibution, CustomDistribution]
probably the first option is a way to go, unless I am missing something that would require introduction of new specs?
|
Thanks @daniil for looking into this. As per usual these are great suggestions.
I'll implement this as done in fdtd. I guess my only comment is that depending on the simulation the amount of data generated during the run could be quite big since we'd be saving every time-step and then post-process only the required steps. On the other hand, once the simulation is done and post-processed, all those temp files are removed, so it is only a temporary problem?
I like this. This may be related to this issue where we want to interpolate data to a mesh. I guess once I resolve that issue we can revisit this? |
3752f68 to
c27caa6
Compare
c27caa6 to
5879960
Compare
momchil-flex
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
The main thing that I think remains (and can't entirely be done now) is imposing some restrictions similar to what we have for fdtd sims. Maybe the only one we can really do now is just setting a limit on the maximum number of time steps in the simulation (and maybe some ball park limits on the time step). Basically users will inevitably create something that has way too many time steps for some reason, unless we limit it.
9b8f8ea to
1982da4
Compare
dbochkov-flexcompute
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! A few minor comments
26f8015 to
87a0b12
Compare
de62190 to
7ae68f3
Compare
Code Coverage SummaryDetailsDiff against developResults for commit: 23b312f Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Changed Files CoverageResults for commit: 23b312f Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Heat capacity is now not a required parameter unless we run unsteady heat
7ae68f3 to
23b312f
Compare
Adding support for unsteady heat simulations.