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 a plot header file for sampling particles #1435

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

marchdf
Copy link
Contributor

@marchdf marchdf commented Jan 8, 2025

Summary

Add a Header file for sampling particles. Long story of why is below.

Pull request type

Please check the type of change introduced:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Checklist

The following is included:

  • new unit-test(s)
  • new regression test(s)
  • documentation for new capability

This PR was tested by running:

  • the unit tests
    • on GPU
    • on CPU
  • the regression tests
    • on GPU
    • on CPU

Additional background

Issue #1417 brought to our attention that Paraview >= 5.12 could not read our sampling particles any more.

This was reported to ParaView here: https://gitlab.kitware.com/paraview/paraview/-/issues/22835.

It turns out that Paraview in this PR started requiring a Header file that was written by the WritePlotFile routine (independently of the particles). In the majority of amrex applications, particles are written out in conjunction with a plot file (and in the same directory) so this is typically not an issue because the plot file header is available. However, the AMR-Wind sampling particles are independent (can be written at a different frequency, etc) so our particle directories did not have a Header file.

This PR adds a Header file to the sampling particle directories. I can now use ParaView 5.13 to viz our particles. The added bonus is that it now recognizes the time stamp associated to those particles.

Screenshot 2025-01-08 at 9 47 48 AM

Tagging @rthedin as interested.

@marchdf
Copy link
Contributor Author

marchdf commented Jan 8, 2025

This does make is so that the sampling_info file I introduced in #1387 is redundant because it contains time information that is also in the Header file. But I think I will keep that sampling_info file since it seems generally a useful place to store additional information.

@marchdf marchdf requested review from mbkuhn and moprak-nrel January 8, 2025 16:50
@marchdf marchdf enabled auto-merge (squash) January 8, 2025 16:58
@rthedin
Copy link
Contributor

rthedin commented Jan 8, 2025

I would request that we do keep the sampling_info file. I have starting adding more info to it on a local-to-me branch to test some things out. Mainly, as of right now, I'm adding a map between the string used for a sampling and the index. That will be needed to ensure post-processing is not dependent on an unmodified input file. I'm planning on doing some more tests and checking in with one of you on how to make my additions to the sampling_info more appropriate (right now it's very much not great coding as I am not familiar enough with AMR-Wind codebase). But it seems like now time is part of the Header file, which is fine to me.

@marchdf marchdf merged commit 498d029 into Exawind:main Jan 8, 2025
15 checks passed
@marchdf marchdf deleted the sampling-header branch January 8, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants