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

adjust_ps=.false. is not tested in cxx code #1343

Open
mt5555 opened this issue Dec 9, 2021 · 6 comments
Open

adjust_ps=.false. is not tested in cxx code #1343

mt5555 opened this issue Dec 9, 2021 · 6 comments
Assignees
Labels

Comments

@mt5555
Copy link
Contributor

mt5555 commented Dec 9, 2021

SCREAM v0.1 (Fortran code) runs with adjust_ps=.false.
CXX code has only been tested with adjust_ps=.true.

E3SM should also migrate to adjust_ps=.false., but this is on hold in order to preserve buggy V2 behavior.

Current CXX vs Fortran bfb tests are in the E3SM repository using standalone HOMME. The CXX code for adjust_ps=.false. needs to be tested. In order to test this in the E3SM repository, we need a way to turn on adjust_ps=.false. in standalone HOMME. some options:

  1. control this feature with a namelist variable. We dont want namelist variables that turn on bugs, so we then have a second PR to remove this once the V2 freeze is lifted and the fix makes it into e3sm.
  2. Test the CXX code by hand, until the V2 code freeze is lifted and this change makes it into E3SM
  3. Test this feature with in the SCREAM repo
  4. Add a #define to allow tests to turn this bugfix on in the E3SM repo
@bartgol
Copy link
Contributor

bartgol commented Sep 14, 2023

@mt5555 we are still hard-coding adjust_ps=true in Hommexx. Do we need to start making this a configurable param in hommexx?

@mt5555
Copy link
Contributor Author

mt5555 commented Sep 14, 2023

Fortran code has:

#ifdef SCREAM
adjust_ps=.false. ! Lagrangian case can support adjusting dp3d or ps
#else
adjust_ps=.true. ! Lagrangian case can support adjusting dp3d or ps
#endif

And thus hommexx should also have adjust_ps=false - not sure how it can pass BFB C++ vs Fortran if adjust_ps=-true?

@PeterCaldwell
Copy link
Contributor

Can you remind me what adjust_ps means?

@mt5555
Copy link
Contributor Author

mt5555 commented Sep 14, 2023

adjust_ps=false: mass fluxes from physics are applied locally (removing the mass from the layer it came from)

adjust_ps=true: mass fluxes are smeared over the whole column and removed from the surface pressure

adjust_ps=true preserves buggy behavior of E3SM. it has minor impacts, so we never attempted to get adjust_ps=.false. this into v3

@mt5555
Copy link
Contributor Author

mt5555 commented Sep 14, 2023

this is one of these nuisance bug fixes. it should have been fixed a long time ago, but the hassle of evaluating it combined with the small impact means it's always a low priority.

It looks like the bugfix is in SCREAM v0.1, but the bugfix never made it into SCREAM v1.

We shouldn't try and get this into CESS, since there are already too many PRs trying to make it in for CESS.

But lets fix this once the CESS runs have started and master is open again

@ndkeen
Copy link
Contributor

ndkeen commented Sep 14, 2023

I would think that changes that are not expected to cause a problem would be OK. We don't need to be BFB right now with previous runs. But will let others decide. Yea I was hoping to never see adjust_ps again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants