Skip to content
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

Snapshot computation #101

Merged
merged 55 commits into from
Jul 25, 2024
Merged

Snapshot computation #101

merged 55 commits into from
Jul 25, 2024

Conversation

tjwsch
Copy link
Collaborator

@tjwsch tjwsch commented May 13, 2024

Partly implementation of snapshot features discussed in #89. This does not yet include the configuration of the parameter space at runtime and a callback interface to invoke postprocessing as discussed in SimTech project PN5-9(II).
Reading parameter information from an hdf5 file and writing snapshot outputs to hdf5 files and the project structure might need more work.

@tjwsch tjwsch self-assigned this May 14, 2024
Copy link
Collaborator Author

@tjwsch tjwsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are some open questions and restructuring of postprocessing is required.

examples/snapshot_example/snapshot-config.json Outdated Show resolved Hide resolved
snapshot/read_write_hdf.py Outdated Show resolved Hide resolved
snapshot/read_write_hdf.py Outdated Show resolved Hide resolved
snapshot/snapshot_computation.py Outdated Show resolved Hide resolved
examples/snapshot_example/.gitignore Outdated Show resolved Hide resolved
examples/snapshot_example/python_dummy/micro_dummy.py Outdated Show resolved Hide resolved
examples/snapshot_example/python_dummy/run_snapshot.py Outdated Show resolved Hide resolved
examples/snapshot_example/snapshot-config.json Outdated Show resolved Hide resolved
snapshot/snapshot_computation.py Outdated Show resolved Hide resolved
snapshot/snapshot_computation.py Outdated Show resolved Hide resolved
@tjwsch tjwsch requested a review from IshaanDesai May 14, 2024 13:33
@tjwsch tjwsch marked this pull request as ready for review May 20, 2024 07:58
Copy link
Member

@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work already 👏 My suggestions are fundamental and hence require some discussion before large implementation changes. Let us discuss here in the comments.

@tjwsch
Copy link
Collaborator Author

tjwsch commented Jun 11, 2024

For large parameter spaces, we need to think about chunking. Everything I have found suggests that, within a dataset, all chunks must have the same size, which complicates matters. A poor choice of chunk size can lead, besides other effects, to files containing empty allocated space. Not knowing the dimensions of a snapshot also complicates finding an optimal chunk size. As subfiling is currently not supported in h5py, writing to one file from multiple processes is not the best idea, and activating MPI support in h5py requires additional steps, writing to a separate file from every process and merging the output files, seems to be the best option available with h5py at this time.
The first option would be to make the user provide the parameter space file in chunks This would allow processes to read only the chunks they require, but slightly complicate the parameter file creation process for the user, and if each file only reads whole chunks leads, to some load imbalances. If chunks are split up between processes this complicates the the decomposition. This would also pre-set a chunk size, which could have advantages and disadvantages.
The second option would be to read the parameter file as a whole and only have chunked output files. Again, the questions of optimal chunk sizes and how to minimize the empty file space are relevant. I think cases exist where each process would have empty allocated space, which would accumulate in the merged file unless the merging does not take place chunk-wise but snapshot-wise, and a new chunk size for the merged file is determined.

@uekerman
Copy link
Member

make the user provide the parameter space file in chunks

This sounds like sth we should better hide from the user.

@IshaanDesai IshaanDesai added the new-feature Adding a new feature label Jul 23, 2024
examples/.gitignore Outdated Show resolved Hide resolved
Copy link
Member

@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍 this is close to merging now. Lets resolve some last comments. Please also remember that documentation for snapshot computation needs to be added. As discussed earlier, we can do this in a separate pull request.

@tjwsch tjwsch merged commit 391969d into develop Jul 25, 2024
10 checks passed
@tjwsch tjwsch deleted the snapshot_computation branch July 25, 2024 07:18
@IshaanDesai IshaanDesai mentioned this pull request Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature Adding a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add configuration options to dictate solving a full-order model for snapshot creation
3 participants