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

Test case that shows problem with Nalu-Tioga link #69

Closed
wants to merge 4 commits into from

Conversation

mbkuhn
Copy link
Contributor

@mbkuhn mbkuhn commented Mar 22, 2024

In Nalu-Wind, the velocity is set to 0.0 and the density is set to 1.0. In AMR-Wind, the velocity is set to [0.1, -0.1, 0.0] and the density is set to 1.5. This shows that when multiple processors are used for Nalu-Wind, there are errors in communicating the fields properly into Nalu-Wind. The iblank field (its boundary shown in black), does not have issues, though, thanks to the recent changes to Nalu-Wind.

Screenshot 2024-03-22 at 2 03 30 PM

@mbkuhn mbkuhn requested a review from itopcuoglu March 22, 2024 20:05
@itopcuoglu itopcuoglu marked this pull request as ready for review March 24, 2024 23:03
Copy link
Contributor

@psakievich psakievich left a comment

Choose a reason for hiding this comment

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

See note about solvers. Also let's not put a mesh file in this repository. Push it to the meshes repo please.

linear_solvers:

- name: solve_scalar
type: tpetra
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we switch these to hypre? These tpetra solver settings are super old and we've not been keeping up with them. I would rather not see another copy of milestone.xml put under version control if we can avoid it.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Mar 25, 2024

Thanks for the input, @psakievich. I wanted a quick way to share it with Ilker, so I put it all here at first. I'll make the changes you requested.

@psakievich
Copy link
Contributor

Sounds good. My requests are only if you want to merge it for a long term test. Otherwise, fine as is.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Mar 25, 2024

It probably would be useful to be added as a long-term part of the repository, but it doesn't fit well as a true regression test because it's just a one time step thing without a real physical meaning. I'll think about it some more.

@psakievich
Copy link
Contributor

I don't think a regression test has to always be physically meaningful. One timestep to verify hole-cutting in parallel hasn't regressed seems like a perfect regression test candidate IMO.

@mbkuhn
Copy link
Contributor Author

mbkuhn commented Aug 5, 2024

Solved with nalu-wind fix related to "set_resolutions" Exawind/nalu-wind#1267

@mbkuhn mbkuhn closed this Aug 5, 2024
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