Skip to content

Conversation

@asalmgren
Copy link
Contributor

Summary

This PR modifies the PPM_NOLIM option to use the code already in AMReX-Hydro instead of code in amr-wind.

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 abl_godunov_nolim ctest

Additional background

Issue Number:

@marchdf marchdf requested review from marchdf and mbkuhn July 25, 2024 15:35
@@ -0,0 +1,36 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remove these 2 scripts?

Copy link
Contributor

@marchdf marchdf left a comment

Choose a reason for hiding this comment

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

Some bash scripts should be removed. I am running the tests locally right now.

@marchdf
Copy link
Contributor

marchdf commented Jul 25, 2024

Thank you for doing this! Really appreciate it

@marchdf
Copy link
Contributor

marchdf commented Jul 25, 2024

I am getting these test failures:

The following tests FAILED:
         11 - abl_godunov_nolim (Failed)
         25 - abl_stable (Failed)
         26 - abl_unstable (Failed)
         27 - abl_unstable_constant_wall_model (Failed)
         28 - abl_unstable_local_wall_model (Failed)
         29 - abl_unstable_schumann_wall_model (Failed)
         40 - uniform_ct_disk (Failed)
         41 - uniform_ct_disk_gaussian (Failed)
         42 - joukowsky_disk (Failed)
         88 - cylinder_refinement (Failed)

They all use ppm_nolim and the diffs are large-ish. For example:

"abl_godunov_nolim" start time: Jul 25 10:07 MDT
Output:
----------------------------------------------------------

            variable name            absolute error            relative error
                                        (||A - B||)         (||A - B||/||A||)
 ----------------------------------------------------------------------------
 level = 0
 density                                          0                         0
 gpx                                 0.002995664903              0.1943711061
 gpy                                 0.002940574774              0.1899127757
 gpz                                0.0008573573962             0.00300011744
 mu_turb                              0.02726567741             0.06064018848
 p                                    0.03946123662           0.0006404818327
 temperature                        1.277840056e-10           4.139172162e-13
 temperature_mueff                    0.08180521275             0.06063954632
 velocityx                            0.05608534872            0.007876552416
 velocityy                            0.06014182843            0.009799012215
 velocityz                           0.002022387944              0.0469507976
 velocity_mueff                       0.02726567741             0.06063883984

Is the implementation of ppm_nolim different in amrex-hydro and amr-wind?

@mbkuhn are you seeing the same on your end?

@asalmgren
Copy link
Contributor Author

asalmgren commented Jul 25, 2024 via email

@marchdf
Copy link
Contributor

marchdf commented Jul 25, 2024

the nolim test passed for me.

A weird feature of the way we do the "tests". If that fcompare flag is off, then the tests "pass" as in they run, but no gold comparison is made. Were you able to set up the golds?

Alternatively, I messed something up.

@asalmgren asalmgren closed this Jul 26, 2024
@asalmgren asalmgren deleted the move_ppm_no_lim branch July 29, 2024 23:38
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