Add support for libyt #209
Conversation
There was a problem hiding this comment.
I haven't yet reviewed the brand new source files but I wanted to submit my review so my comments about the docs & where the in situ analysis is called can be viewed & discussed by all. Once the comments from @dcollins4096 and I have been addressed I want to try running the in situ analysis myself before approving, as I noted in a comment on the new doc page. Super excited about this feature and I want to make sure its broadly accessible!
| * **How to activate in situ Python analysis process?** | ||
|
|
||
| The full process is encapsulated inside ``CallInSitulibyt`` function in ``src/enzo/CallInSitulibyt.C``. | ||
| We can put this function everywhere we want in Enzo to start in situ analysis. |
There was a problem hiding this comment.
The existing uses of CallInSitulibyt are indeed only in EvolveLevel and after the recursive loop. Given that the simulation is supposed to finish evolution before handing over data, this makes sense. I don't think CallInSitulibyt should (much less can) be added to any other location because of this as it would be unwise to allow data to be passed to in situ analysis while the level hierarchy is still being evolved. Though, see my notes on EvolveLevel.C for my thoughts on its current placement.
Overall, I don't think the docs should advertise that CallInSitulibyt can be added anywhere else than where it already is.
| #ifdef USE_LIBYT | ||
| LCAPERF_START("CallInSitulibyt"); | ||
| //CallEmptyInteractivePython(LevelArray, MetaData, level, 0); | ||
| CallInSitulibyt(LevelArray, MetaData, level, 0); | ||
| LCAPERF_STOP("CallInSitulibyt"); | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Why is CallInSitulibyt called before flux correction? Would it be better to call it from EvolveHierarchy? It's already being called after EvolveLevel's recursive loop.
There was a problem hiding this comment.
I believe I did this exclusively to mimic the behavior of the existing python in situ stuff. I think as long as it's able to be called from either the top or bottom of the W cycle it's fine to call it.
There was a problem hiding this comment.
Will calling it before flux correction have any strange consequences on cells at the boundaries of grids with different resolution? I know yt will automatically use data at the finest resolution (unless you do a lot of manual intervention)
There was a problem hiding this comment.
I'm not entirely sure, nor which would be more desired. I think I was attempting to replicate as best I could the interpolated output machinery.
| If an error occurred when running inline Python functions or Enzo detects ``LIBYT_STOP`` file, then it will enter reloading script phase. | ||
| Document about ``yt_run_ReloadScript`` is `here <https://libyt.readthedocs.io/en/latest/libyt-api/yt_run_reloadscript.html>`__. | ||
|
|
||
| * **How to reload a script?** |
There was a problem hiding this comment.
It's not clear to me why I would want to use this feature; an example use case would really help. Even just a few sentences.
|
|
||
| // TODO: yt_run_Function and yt_run_FunctionArguments | ||
| // Put yt_run_Function and yt_run_FunctionArguments here | ||
|
|
There was a problem hiding this comment.
Agree with @dcollins4096; ideally I'd like to try running the in situ analysis myself before approving this PR. That way I can identify gaps in the docs.
Co-authored-by: Claire Kopenhafer <[email protected]>
Co-authored-by: Claire Kopenhafer <[email protected]>
Fix Enzo Doc
Because libyt also defines int, float... in its header, explicitly extern the global variable.
Make Python Script Name a Global Variable
Fix Enzo PR
Fix Enzo PR
|
Hi @clairekope and @dcollins4096, The majority of the comments are fixed, except these:
The docs are all up-to-date, I would be really happy if you could give me some feedback about |
Remove advertising CallInSitulibyt can be called anywhere.
This adds support for libyt to enzo. This week in San Diego, @cindytsai will talk more about it, and she can say more here, but for posterity here are a few bits:
libytconfiguration option; outside of that, it should not change compilationlibyt, counterintuitively, does not require yt, but instead manages calling Python (3) for any purpose, supplying the appropriate information from Enzo. Python is not directly linked, but instead connected vialibyt.libytas well.There are a few things left, but as it stands this can be run:
deletethe fields afterwardlibyt, which isn't directly mappable to Enzo's hierarchy system, as it would require a global lookup table for grids. I'm working on a way around this.