Skip to content

boundary condition bug fixes and RUC LSM precision fix#156

Merged
SamuelTrahanNOAA merged 4 commits into
NOAA-GSL:RRFS_devfrom
SamuelTrahanNOAA:RRFS_dev_debug
Sep 28, 2022
Merged

boundary condition bug fixes and RUC LSM precision fix#156
SamuelTrahanNOAA merged 4 commits into
NOAA-GSL:RRFS_devfrom
SamuelTrahanNOAA:RRFS_dev_debug

Conversation

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator

This fixes many bugs in regional boundary conditions, and a precision issue in module_sf_ruclsm. It is a copy of a community PR, with a different destination (RRFS_dev).

Community PR:

NOAA-EMC#587

Dependencies:

NOAA-GSL/ccpp-physics#163
NOAA-GSL/GFDL_atmos_cubed_sphere#12

@hu5970
Copy link
Copy Markdown

hu5970 commented Sep 23, 2022

@SamuelTrahanNOAA Other than updates to the tests, this PR changed two files:
model/fv_regional_bc.F90
physics/module_sf_ruclsm.F90

Right?

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

@SamuelTrahanNOAA Other than updates to the tests, this PR changed two files: model/fv_regional_bc.F90 physics/module_sf_ruclsm.F90

Right?

Correct.

@hu5970
Copy link
Copy Markdown

hu5970 commented Sep 23, 2022

@SamuelTrahanNOAA Other than updates to the tests, this PR changed two files: model/fv_regional_bc.F90 physics/module_sf_ruclsm.F90
Right?

Correct.

Great, I will update this two files in my retro and run retro again. Thanks, Ming

@hu5970
Copy link
Copy Markdown

hu5970 commented Sep 23, 2022

@SamuelTrahanNOAA Could you germ this PR and then let Haiqin update his PR? Thanks, Ming

@hu5970
Copy link
Copy Markdown

hu5970 commented Sep 23, 2022

@haiqinli Please update your PR based on this one and finalize your PR as soon as possible. Thanks, Ming

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

@SamuelTrahanNOAA Could you germ this PR and then let Haiqin update his PR? Thanks, Ming

I'm waiting for review from someone in the know before germing the PR. I've never even looked at fv_regional_bc.F90 until Monday, so I want someone who understands it to confirm I'm not just breaking things further.

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

SamuelTrahanNOAA commented Sep 23, 2022

@haiqinli Please update your PR based on this one and finalize your PR as soon as possible. Thanks, Ming

I have bug fixes to put on top of @haiqinli's changes. The sooner he finalizes those, the sooner I can apply my bug fixes.

He may be able to fix some of the issues himself if he applies these changes and runs in debug and 2threads modes. Things crashed sometimes when I changed the thread count. I have fixes for that, and other problems.

Edit: Ravan has reported threading-related crashes in other situations when he disables blocks of code. (Blocks that shouldn't cause crashes.) I'm hoping my threading-related bug fixes will correct that as well.

@haiqinli
Copy link
Copy Markdown
Collaborator

@hu5970 Yes, the updates of smoke code in my PR is done. I will include this boundary condition update into my PR and finalize it soon.

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

@haiqinli Remember to include the module_sf_ruclsm fix too, otherwise the debug mode will fail due to a precision issue.

@haiqinli
Copy link
Copy Markdown
Collaborator

@SamuelTrahanNOAA Sure, I am doing a quick test run with my PR to include the module_sf_ruclsm fix.

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

@haiqinli If you can also compile with "-DDEBUG=ON" and get that to run with smoke, it'll save me a lot of time.

@haiqinli
Copy link
Copy Markdown
Collaborator

@SamuelTrahanNOAA Sure, I will compile with "-DDEBUG=ON".

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

SamuelTrahanNOAA commented Sep 23, 2022

@SamuelTrahanNOAA Sure, I will compile with "-DDEBUG=ON".

The fixes for debug, restart, and threading are in the bugfix/rrfs-debug-mode branch of my ccpp-physics fork. They're also necessary to get smoke to run without crashing on a restart. That is based on the community version of smoke, but most of the changes should be valid for the current version. You won't be able to test the threading properly since the RRFS_dev does not match its control in 2threads mode. We will have to trust to fate that the thread bugfixes will fix this newer smoke code too, as there is no other way to test it. However, we can test debug and restart.

https://github.com/SamuelTrahanNOAA/ccpp-physics/tree/bugfix/rrfs-debug-mode

git clone --branch bugfix/rrfs-debug-mode --recursive https://github.com/SamuelTrahanNOAA/ccpp-physics rrfs-debug-mode

Copy link
Copy Markdown

@tanyasmirnova tanyasmirnova left a comment

Choose a reason for hiding this comment

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

@SamuelTrahanNOAA Ai approve the workaround in RUC LSM. Thank you, Sam, for discovering this problem.
I am not familiar with the fv_regional_bc.F90 code, but the changes look reasonable to me, therefore, I approve these changes.

@haiqinli
Copy link
Copy Markdown
Collaborator

@hu5970 @SamuelTrahanNOAA My PR is finalized with smoke, ruc_lsm, and regional_bc updates, and works well in debug mode.

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

@haiqinli Where is the test case that you want me to turn into regression tests?

@hu5970
Copy link
Copy Markdown

hu5970 commented Sep 23, 2022

@SamuelTrahanNOAA @haiqinli The latest CONUS 13km case should be used for smoe/dust regression test by turning option on, right?

@haiqinli
Copy link
Copy Markdown
Collaborator

@hu5970 If there is only one CONUS13km regression test run case, I suggest turning on smoke/dust option.

@SamuelTrahanNOAA
Copy link
Copy Markdown
Collaborator Author

All three will be present (no smoke, smoke, radar_tten)

@SamuelTrahanNOAA SamuelTrahanNOAA changed the title boundary condition bug fixes and module_sf_ruclsm precision fix boundary condition bug fixes and RUC LSM precision fix Sep 23, 2022
@SamuelTrahanNOAA SamuelTrahanNOAA merged commit 2d4dac2 into NOAA-GSL:RRFS_dev Sep 28, 2022
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.

7 participants