remove ParthenonInit and add docs #930
Conversation
Yurlungur
added
documentation
| ParthenonStatus ParthenonManager::ParthenonInitEnv(int argc, char *argv[]) { | ||
| if (called_init_env_) { | ||
| PARTHENON_THROW("ParthenonInitEnv called twice!"); |
| if (called_init_packages_and_mesh_) { | ||
| PARTHENON_THROW("Called ParthenonInitPackagesAndMesh twice!"); | ||
| } | ||
| called_init_packages_and_mesh_ = true; | ||
|
|
There was a problem hiding this comment.
LGTM. I'd have done static vars with function scope but this is more flexible. Could see e.g. erroring in driver.Execute if init wasn't called, or other hand-holding like that if there are common errors. It also allows multiple ParthenonManagers I guess, though I'm not sure MPI & Kokkos would react well to that anyway.
For my own edification: are there other reasons to prefer members?
To be honest, I didn't think about it very hard. Members seemed natural and easy to reason about, since the object is already stateful. If people prefer statics, that's fine with me. |
|
Ping someone from AthenaPK @BenWibking @forrestglines @pgrete . Can we merge this in? |
|
|
||
| ParthenonStatus ParthenonManager::ParthenonInitEnv(int argc, char *argv[]) { | ||
| // initialize MPI | ||
| #ifdef MPI_PARALLEL | ||
| if (MPI_SUCCESS != MPI_Init(&argc, &argv)) { |
There was a problem hiding this comment.
At some point we'd like to optionally start withMPI_Init_thread which is apparently required by HDF5 subfiling. Another time and PR though
PR Summary
Based on the discussion in #929 , it seems like it might be a good idea to remove the
ParthenonManager::ParthenonInitfunction. This PR does so. I also add documentation onParthenonManager, which I couldn't otherwise find.This is a breaking change. I would like the go-ahead from downstream codes before merging this.
PR Checklist