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

Ensure consistent ghost cells for terrain fields #1353

Merged
merged 10 commits into from
Nov 18, 2024

Conversation

marchdf
Copy link
Contributor

@marchdf marchdf commented Nov 18, 2024

Summary

Ensure consistent ghost cells for terrain. Make sure that the interior ghost cells of the int fields are properly filled. All tests pass and I don't expect any diffs (because we don't use these cells normally). However, if one were to use a sampler or a refiner on these fields, the ghost cells won't be filled and the interpolater (for the samplers) and the grad refinement (for the refiner) won't see the right values in the -/+1 cells.

I also moved the terrain field inits to the actual function we like to use for this and not post init. This is to make sure that the mesh creation sees valid values in those fields (useful for refinement).

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

@marchdf marchdf marked this pull request as ready for review November 18, 2024 16:28
@marchdf marchdf changed the title Ensure consistent ghost cells for terrain Ensure consistent ghost cells for terrain fields Nov 18, 2024
@marchdf
Copy link
Contributor Author

marchdf commented Nov 18, 2024

One thing to note... for int fields we don't do actual domain BCs so I am only filling the interior BCs for the int fields. One issue with this is that the refiners and samplers, if they happen at a domain BC, will see a default value. The user can be careful about that and fill the domain ghost cells appropriately. Or I can code in the infracstructure for something like an hoextrap for int fields. Thoughts?

@hgopalan
Copy link
Contributor

hoextrap should be fine since at the domain boundaries the terrain gets smoothed out close to zero.

@marchdf marchdf marked this pull request as draft November 18, 2024 17:07
@marchdf
Copy link
Contributor Author

marchdf commented Nov 18, 2024

This is an example of what I don't want to have happen. Basically it tagged everything in the boundary because the boundary values are "wrong".

Screenshot 2024-11-18 at 10 25 40 AM

@marchdf
Copy link
Contributor Author

marchdf commented Nov 18, 2024

Ok I found a better way to do this without fillpatch/fillboundary. Basically I am having the parallelfor loops fill the ghost values properly with what they should be initialized too. It looks like a lot of diffs but I basically just moved code from post_init to initialize_fields. And then used fused mfiter loops that loop on ghost cells as well.

@marchdf
Copy link
Contributor Author

marchdf commented Nov 18, 2024

Now my refinement looks like this. Which is what I wanted.

Screenshot 2024-11-18 at 10 46 41 AM

@marchdf marchdf mentioned this pull request Nov 18, 2024
14 tasks
@marchdf marchdf marked this pull request as ready for review November 18, 2024 17:51
@marchdf marchdf enabled auto-merge (squash) November 18, 2024 18:10
@marchdf marchdf merged commit 6370680 into Exawind:main Nov 18, 2024
15 checks passed
@marchdf marchdf deleted the terrain-tweak branch December 3, 2024 22:49
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.

2 participants