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

Add fix for reading sfc fluxes for DEPHYv1 format #428

Closed
wants to merge 8 commits into from

Conversation

hertneky
Copy link
Collaborator

These changes address a bug found when running SCM with DEHPYv1 cases where surface fluxes are prescribed. This addresses Issue #427

@@ -490,10 +490,10 @@ def setup_rundir(self):
if input_type == 1:
#open the case data file and read the surfaceForcing global attribute
case_data_dir = case_nml['case_config']['case_data_dir']
nc_fid = Dataset(os.path.join(SCM_ROOT, case_data_dir) + '/' + self._case + '_SCM_driver.nc' , 'r')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need these changes to lines 493-494? (Just the change to line 496)

Copy link
Collaborator Author

@hertneky hertneky Jan 25, 2024

Choose a reason for hiding this comment

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

@dustinswales Thanks, I reverted the L493 change. I just tested and it is working fine (note I made that change awhile back due to some random issue reading the file). Thinking about it, I wonder if surface_forcing_moisture should also be considered here, since they can be set independently. I'm not sure how the SCM would react with them set differently without looking more at the code.

Copy link
Collaborator Author

@hertneky hertneky Feb 23, 2024

Choose a reason for hiding this comment

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

@dustinswales Revisiting this reverted change. It looks like changes are needed here otherwise the fluxes are not read in correctly. case_nml['case_config']['case_data_dir'] is empty, so either it needs to be assigned the path somewhere above, OR remove L492 and modify L493 case_data_dir > self._case_data_dir. This only shows up when prescribing sfc_fluxes. While the case will appear to run normally without a fix, it will run the default suite and not the *_ps version since nothing is getting read correctly here.

@egrell
Copy link
Collaborator

egrell commented Jan 25, 2024 via email

@dustinswales
Copy link
Collaborator

For the MPACE case, the name of the attribute was 'surface_forcing_temp'. So I changed line 494 and 496, but not 493. if input_type == 1: #open the case data file and read the surfaceForcing global attribute case_data_dir = case_nml['case_config']['case_data_dir'] nc_fid = Dataset(os.path.join(SCM_ROOT, case_data_dir) + '/' + self._case + '_SCM_driver.nc' , 'r') surfaceForcing = nc_fid.getncattr('surface_forcing_temp') nc_fid.close() if (surfaceForcing.lower() == 'flux' or surfaceForcing.lower() == 'surface_flux'): surface_flux_spec = True

On Thu, Jan 25, 2024 at 12:51 PM Dustin Swales @.> wrote: @.* requested changes on this pull request. ------------------------------ In scm/src/run_scm.py <#428 (comment)>: > @@ -490,10 +490,10 @@ def setup_rundir(self): if input_type == 1: #open the case data file and read the surfaceForcing global attribute case_data_dir = case_nml['case_config']['case_data_dir'] - nc_fid = Dataset(os.path.join(SCM_ROOT, case_data_dir) + '/' + self._case + '_SCM_driver.nc' , 'r') I don't think we need these changes to lines 493-494? (Just the change to line 496) — Reply to this email directly, view it on GitHub <#428 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADMPXGJDL2WPVOZQWUO75QDYQKZULAVCNFSM6AAAAABCJQFI6OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNBUGU3DQNJWGA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

@egrell Sorry, I see in the DEPHY standard that its "surface_forcing_temp".

@scrasmussen scrasmussen added the CCPP v7 Needed for CCPP v7 public release label Feb 15, 2024
@hertneky hertneky changed the title Add fix for reading sfc fluxes for DHEPHYv1 format Add fix for reading sfc fluxes for DEPHYv1 format Feb 23, 2024
@grantfirl
Copy link
Collaborator

@hertneky Is this ready? I'm combining several PRs into #459 for ease of merging.

grantfirl added a commit to grantfirl/ccpp-scm that referenced this pull request Apr 11, 2024
@grantfirl
Copy link
Collaborator

Closing due to changes being combined into #459.

@grantfirl grantfirl closed this Apr 11, 2024
@hertneky
Copy link
Collaborator Author

@grantfirl The only remaining question was...do we need to do something different since in DEPHY, the surface flux flags are separate > "surface_forcing_temp" and "surface_forcing_moist". Right now, only "surface_forcing_temp" is checked to determine whether surface fluxes are prescribed or not.

scrasmussen pushed a commit to scrasmussen/ccpp-scm that referenced this pull request May 22, 2024
mzhangw added a commit to mzhangw/ccpp-scm that referenced this pull request May 22, 2024
* upstream/main:
  Add GP SDFs/NMLs to SCM
  minor fix/cleanup
  Add namelist option and sigmab (tracer) for HR3
  add SCM_RRFS_v1 SDF, nml, tracer file; update suite_info; update scm_type_defs to recgonize new tracers
  add script to download and stage climatological aerosol data and fix typo in HR3 namelist
  merge in changes from NCAR#428 from @hertneky
  add CITATION.cff file from @scrasmussen from NCAR#458
  merge changes from @hertneky in NCAR#446
  add HR3 suite and physics namelist from NCAR#445 from @bluefinweiwei
  changes from @bluefinweiwei for PR#419
  Revert submodules
  Update physics
  Address reviewers comments
  update ccpp/physics submodule pointer
  Moving documentation to ReadTheDocs (NCAR#438)
  Synced physics
  update ccpp/physics submodule
  update ccpp/framework
  update CCPP_typedefs to work with PR#181
  update ccpp/physics and .gitmodules
  Added step to skip gridpoint in ensemble if error during case generation
  Added utility script to create input file containing replay points
  Omission from previous commit
  Add capability to use input file with longitude and latitude lists
  update ccpp/physics submodule
  update .gitmodules for whitespace change removal
  update ccpp/physics and .gitmodules
  update ccpp/physics and .gitmodules
  update ccpp/physics submodule pointer
  point .gitmodules back to my branch for CI testing
  update ccpp/physics and .gitmodules
  update ccpp/physics and .gitmodules
  update ccpp/physics and .gitmodules
  Cleanup/Housekeeping. Remove unnecessary interpolation step
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Revert change to CMakeLists
  Revert change to CMakeLists
  Revert change to CMakeLists
  Revert change to CMakeLists
  Update CI
  Update CI
  Add file for Nvidia RTS
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update cmakelists
  Updated CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CMakeLists for Nvidia support
  Update CMakeLists for Nvidia support
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Update CI
  Foundatio for Nvidia CI script
  Synced physics
  Small cahnge from physics needed for replay
  Replay with RUC and NoahMP LSM working
  Some changes for Man to test
  If using model initial conditions and forcing from the UFS (e.g. Replay mode) with an active LSM, the SCM performs a cold-start initialization for the LSM. That is, UFS replay mode currently support cases generated from UFS Restart files (e.g spun up (warm) LSM parameters)
  update ccpp/physics and revert .gitmodules
  update ccpp/physics submodule and revert .gitmodules
  changes to work with ufs/dev PR#155
  update for ufs/dev PR#94
  update ccpp/physics and .gitmodules
  update ccpp/physics and .gitmodules after testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug CCPP v7 Needed for CCPP v7 public release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants