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

add PlotFileData bindings #320

Merged
merged 23 commits into from
Jul 1, 2024
Merged

Conversation

BenWibking
Copy link
Contributor

@BenWibking BenWibking commented May 19, 2024

Enable reading plotfiles with pyAMReX. Also adds a script in tools/ to create images from plotfiles.

Example use-case: https://github.com/BenWibking/plotfile-viewer.

Closes #317.

@ax3l ax3l requested review from ax3l and atmyers May 30, 2024 06:03
@ax3l ax3l added the enhancement New feature or request label May 30, 2024
@ax3l
Copy link
Member

ax3l commented May 30, 2024

@BenWibking thanks a lot, looking good! Ping us when you are ready with the unit test and we can include it maybe in this month's release even?

@BenWibking
Copy link
Contributor Author

BenWibking commented May 30, 2024

@ax3l I've added a unit test to read a 3D plotfile with a single level. I put the binary data in the PR, but I can move that to some more appropriate place if needed.

When I try to add a test for a 2D plotfile, I get this error:

tests/test_plotfiledata_2d.py:8: in <module>
    import amrex.space2d as amr
../myenv/lib/python3.12/site-packages/amrex/space2d/__init__.py:22: in <module>
    from . import amrex_2d_pybind
E   ImportError: generic_type: type "AMReX" is already registered!

Do 2D tests need to have some special configuration?

@BenWibking BenWibking marked this pull request as ready for review May 31, 2024 19:50
@BenWibking BenWibking changed the title [WIP] add PlotFileData bindings add PlotFileData bindings May 31, 2024
@BenWibking
Copy link
Contributor Author

@ax3l Aside from the 2D test issue, everything should be working now.

tests/projz04000/Header Outdated Show resolved Hide resolved
tools/plot_plotfile_2d.py Show resolved Hide resolved
tools/plot_plotfile_2d.py Show resolved Hide resolved
tools/plot_plotfile_2d.py Show resolved Hide resolved
tests/test_plotfiledata.py Show resolved Hide resolved
tests/test_plotfiledata.py Show resolved Hide resolved
Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
@ax3l ax3l self-assigned this Jun 12, 2024
tests/test_plotfiledata.py Outdated Show resolved Hide resolved
tools/plot_plotfile_2d.py Fixed Show fixed Hide fixed
tests/test_plotfiledata.py Fixed Show fixed Hide fixed
@baperry2
Copy link

This is awesome! I was hoping for this capability. In general, to use pyamrex for pre/postprocessing workflows having as much I/O capability as possible is super helpful.

Another example would be reading/writing individual FAB data (appears to be commented out here:

), which would have been useful for something I was looking at doing a while ago, but am not longer planning on.

@BenWibking
Copy link
Contributor Author

This is awesome! I was hoping for this capability. In general, to use pyamrex for pre/postprocessing workflows having as much I/O capability as possible is super helpful.

Another example would be reading/writing individual FAB data (appears to be commented out here:

), which would have been useful for something I was looking at doing a while ago, but am not longer planning on.

That would be useful. I'm not sure why that's commented out. Maybe @ax3l knows?

In the future, I would like to add the ability to lazy-load FABs for out-of-core analysis, but that might require additional changes in AMReX.

@ax3l
Copy link
Member

ax3l commented Jun 18, 2024

Awesome, yes those function should be added, too, but maybe in a separate PR.

There are some comments in #82 and tests/test_farraybox.py how to bind C++ iostreams and Python io. I simply had no time to test it and no need yet, but PRs are very welcome on it (with a test please) 🙏

@ax3l
Copy link
Member

ax3l commented Jul 1, 2024

Hi @BenWibking , do you have a chance to finish this for the release slayed for today? :)

@BenWibking
Copy link
Contributor Author

@ax3l Thanks for the reminder. Let me know how this looks.

@BenWibking
Copy link
Contributor Author

BenWibking commented Jul 1, 2024

It looks like the CI failed due to "no disk space left on device":

  Error: Error running analysis for cpp: Encountered a fatal error while running "/opt/hostedtoolcache/CodeQL/2.17.6/x64/codeql/codeql database run-queries --ram=14567 --threads=4 /home/runner/work/_temp/codeql_databases/cpp --expect-discarded-cache --min-disk-free=1024 -v --intra-layer-parallelism". Exit code was 2 and error was: A fatal error occurred: Could not write a relation to a file
  (eventual cause: IOException "No space left on device") See the logs for more details. For more information, see https://gh.io/troubleshooting-code-scanning/out-of-disk-or-memory

Copy link
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

This looks great, thank you! ✨

@ax3l
Copy link
Member

ax3l commented Jul 1, 2024

The CI error is from static code analysis. I think @WeiqunZhang know how we can exclude part of the code (the AMReX part) to get it into the limits of CI?

@ax3l ax3l merged commit b13f68c into AMReX-Codes:development Jul 1, 2024
18 of 19 checks passed
@ax3l
Copy link
Member

ax3l commented Jul 2, 2024

Hi @BenWibking,

Thank you for your contribution! Please feel free to add yourself to our .zenodo.json so we can credit you in releases.

@BenWibking BenWibking deleted the add-plotfiledata branch July 2, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bindings for PlotFileData
3 participants