Exawind / nalu-wind

Solver for wind farm simulations targeting exascale computational platforms
https://nalu-wind.readthedocs.io
Other
122 stars 83 forks source link

Multiphase stabilization and mass-momentum consistency updates #1265

Closed wjhorne closed 1 month ago

wjhorne commented 2 months ago

This PR contains all of the multiphase work that was used to successfully run hybrid cases of waves passing over solid bodies. The main changes include

wjhorne commented 2 months ago

@psakievich Do you know anyone that could take a look at this to start getting the ball rolling on getting it in?

psakievich commented 2 months ago

I will try to take a look but I'm not sure how timely I can be. @BumseokLee would you also be willing to take a look? @mbkuhn your input would also be helpful. Also @rcknaus if you have some spare time your input is always appreciated. We have a P/T you can charge for your time if needed.

BumseokLee commented 2 months ago

Hi Phil, I am on a business trip until July 6. Also, I don’t have experiences with the multiphase stabilization or mass-momentum consistency in Nalu-Wind. I think it would be better to assign someone else to the work. Bumseok

psakievich commented 2 months ago

@wjhorne will you add some regression tests that utilize the user functions you've added?

wjhorne commented 1 month ago

@wjhorne will you add some regression tests that utilize the user functions you've added?

We have fully-coupled tests that utilize the user functions. The only exception is the droplet case, which is also covered by VOFDroplet in our regression tests.

wjhorne commented 1 month ago

I pushed in changes addressing all of the review here. Let me know how it looks and I'll keep iterating

mbkuhn commented 1 month ago

I'm getting a weird error with this PR. I'm using it to run these reg tests. When tpetra is used for the velocity solver, the reg tests run fine. But with hypre instead, I get the error "terminating due to uncaught exception of type std::runtime_error: HypreLinearSystem::sumInto not yet implemented for (NON) overset constaint algorithms. Exiting." What makes this strange is the other (single-phase) reg tests use hypre for velocity with no issues, and these two-phase cases run fine with hypre for the volume_of_fluid equation.

mbkuhn commented 1 month ago

I'm getting a weird error with this PR...

This doesn't have to do with any of the changes in the PR. Running one of the cases with master (and turning off the unavailable user function sloshing_tank) still leads to the error. It seems to be a problem with the velocity + volume_of_fluid equations together, not separately (because the zalesak reg test runs fine).

psakievich commented 1 month ago

@mbkuhndo those single phase tests use overset? Here is where the error comes from: https://github.com/Exawind/nalu-wind/blob/53f493954891937db39876452af46dbf3ce3811c/src/HypreLinearSystem.C#L2399-L2403

looking at the calls of sumInto it seems it would hit with the computing overset pressure call for this case here: https://github.com/Exawind/nalu-wind/blob/53f493954891937db39876452af46dbf3ce3811c/src/overset/AssembleOversetPressureAlgorithm.C#L184-L185

Also worth checking if the other cases are using a segregated velocity solve. It may use a different path. Looks like the HypreUVW solver has this as a null function. https://github.com/Exawind/nalu-wind/blob/53f493954891937db39876452af46dbf3ce3811c/include/HypreUVWLinearSystem.h#L171-L193

I don't have a great mental model of how all these go together though.

mbkuhn commented 1 month ago

Update: the problem is not the VOF algorithm or even overset. It's the buoyancy term.

None of the other overset reg tests contain the buoyancy term, and typical runs with nalu-wind tend to employ the buoyancy_boussinesq term, which has no issue with hypre solvers. To check, I ran the problem exawind-driver PR reg tests with the buoyancy term turned off (no error). I looked into the nalu-wind reg tests to find tests with the plain "buoyancy" source term, I replaced the tpetra solver with hypre for velocity, and these tests generated the same sumInto error. These were the VOFDroplet and periodic3dEdge reg tests. All of these checks were performed with the master branch of nalu-wind, not this PR.

Should I make an issue for this?

wjhorne commented 1 month ago

@psakievich I believe with the latest push it should be good to go for a merge. Let me know what you think

mbkuhn commented 1 month ago

We have fully-coupled tests that utilize the user functions. The only exception is the droplet case, which is also covered by VOFDroplet in our regression tests.

SloshingTank is/will be covered by reg tests for exawind-driver; I added a VOFInertialDroplet case so the droplet velocity user function would be covered.