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

Changing Freesurface post-processing routine to a Sampler #1187

Merged
merged 31 commits into from
Aug 20, 2024

Conversation

mbkuhn
Copy link
Contributor

@mbkuhn mbkuhn commented Aug 6, 2024

Summary

FreeSurface is designed to find the location of the air-water interface in simulations with vof. This PR converts that capability to a Sampler type, enabling sampling of fields at the interface and native output with AMReX particles.

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

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 6, 2024

I plan to change the names of the files from FreeSurface to FreeSurfaceSampler, but that would make it much harder to review. So I'm saving that for last.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 6, 2024

Also, I have python scripts to read in the data for every sampler type (ascii, native, and netcdf). I think these should be added to tools, and they'd be set up to read the output from the reg test that I added this sampler too. Thoughts?

Figuring out the layout of any of these types took me some time, and having an example should be helpful.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 6, 2024

Screenshot 2024-08-06 at 11 39 27 AM
Screenshot 2024-08-06 at 11 22 02 AM

some evidence it works

@marchdf
Copy link
Contributor

marchdf commented Aug 7, 2024

Examples are useful! Maybe better in the documentation? I worry about adding python scripts that could go out of date because we can't test them automatically. We can chat more about that.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 8, 2024

Examples are useful! Maybe better in the documentation? I worry about adding python scripts that could go out of date because we can't test them automatically. We can chat more about that.

That's a good idea. I learned that jupyter notebooks can be included in sphinx documentation, so I might try that.

@mbkuhn mbkuhn marked this pull request as ready for review August 9, 2024 20:43
@mbkuhn mbkuhn requested a review from marchdf August 9, 2024 20:43
@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 9, 2024

I set up the docs so that they actually run the jupyter notebooks and generate the plots, and the notebooks are displayed in the docs for easy copy-paste.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 12, 2024

Here's how the docs look with these changes.

docs_with_notebooks.tar.gz

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 12, 2024

@jrood-nrel do you think you could take a look at what I'm doing with the docs, namely, changing the conf.py and docs.yml files?

@moprak-nrel do you have time to look over the stuff in the source code?

@jrood-nrel
Copy link
Contributor

conf.py looks fine. I think I would object to committing several binary files here though.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 13, 2024

conf.py looks fine. I think I would object to committing several binary files here though.

Thanks for the feedback. I'll talk to Marc about what's the best way to incorporate this stuff.

@mbkuhn mbkuhn enabled auto-merge (squash) August 20, 2024 03:06
@mbkuhn mbkuhn merged commit f867288 into Exawind:main Aug 20, 2024
13 of 14 checks passed
@mbkuhn mbkuhn deleted the freesurface_improvements branch August 22, 2024 17:23
mbkuhn added a commit to mbkuhn/amr-wind that referenced this pull request Aug 22, 2024
* remove incomplete, unused python script
* add FreeSurfaceSampler to reg test
* documentation
* python scripts that pass with ctest (to be added later)
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