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

[develop] Integrate 'online-cmaq' branch into 'develop' branch: Part 1, jjob and ex- scripts #536

Merged
merged 3 commits into from
Jan 17, 2023

Conversation

chan-hoo
Copy link
Collaborator

DESCRIPTION OF CHANGES:

Currently, the Online-CMAQ for air quality modeling is separately managed in the 'online-cmaq' branch of the authoritative UFS SRW App repository. The main target of the current 'online-cmaq' branch is the implementation of AQM ver.7 and delivery to NCO in late March. Therefore, it has been developed and tested on WCOSS2 and Hera. The Online-CMAQ checks out the same ufs weather model as the 'develop' branch. However, since it couples another component 'AQM', the compile process is different from that of the 'develop' branch.

This 'online-cmaq' branch is integrated into the 'develop' branch with this PR and the following two PRs. For the convenience of the reviewers, we split the 'online-cmaq' branch into three parts as follows:

  • Part 1 (this PR): j-job and ex- scripts
  • Part 2: build scripts and module files
  • Part 3: workflow set-up, generation, run scripts

In Part1, we add extra j-job and ex- scripts only for Online-CMAQ. As these scripts are totally independent from the 'develop' branch, they do not affect any existing configurations and test cases at all.
In part2, we will modify the 'devbuild.sh' script and add 'Externals_AQM.cfg' to compile the coupled ufs weather model and extra external components for AQM. Same as Part 1, Part 2 will not affect any existing configurations and test cases.
In part3, we will update workflow set-up, generation, run scripts for Online-CMAQ as well as FV3-LAM (current 'develop' branch). New test cases should be added in this step.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

TESTS CONDUCTED:

  • WE2E tests:
    grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp_regional
    grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
    grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
    grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_HRRR
    grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_HRRR_suite_RRFS_v1beta
    grid_RRFS_CONUS_3km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15_thompson_mynn_lam3km
    grid_RRFS_CONUScompact_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
    grid_RRFS_CONUS_13km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
    grid_RRFS_NA_3km_ics_FV3GFS_lbcs_FV3GFS_suite_RRFS_v1beta
    MET_verification
    MET_ensemble_verification
    community_ensemble_2mems_stoch

  • hera.intel

  • orion.intel

  • cheyenne.intel

  • cheyenne.gnu

  • gaea.intel

  • jet.intel

  • wcoss2.intel

  • NOAA Cloud (indicate which platform)

  • Jenkins

  • fundamental test suite

  • comprehensive tests (specify which if a subset was used)

ISSUE:

Fixes issue mentioned in #534

CHECKLIST

  • My code follows the style guidelines in the Contributor's Guide
  • I have performed a self-review of my own code using the Code Reviewer's Guide
  • I have commented my code, particularly in hard-to-understand areas
  • My changes need updates to the documentation. I have made corresponding changes to the documentation
  • My changes do not require updates to the documentation (explain).
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

Copy link
Collaborator

@danielabdi-noaa danielabdi-noaa left a comment

Choose a reason for hiding this comment

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

@chan-hoo This is impressive work!! I have left a few minor generic comments related to the style of the code -- cant comment about what the tasks do. I like that the scripts did not forget about the NCO standards.
I guess in the second/third round of PRs, you will have some benchmarks to run, so will help with testing it then!

jobs/JREGIONAL_AQM_ICS Outdated Show resolved Hide resolved
jobs/JREGIONAL_BIAS_CORRECTION_O3 Outdated Show resolved Hide resolved
jobs/JREGIONAL_BIAS_CORRECTION_O3 Outdated Show resolved Hide resolved
jobs/JREGIONAL_AQM_LBCS Show resolved Hide resolved
jobs/JREGIONAL_BIAS_CORRECTION_PM25 Outdated Show resolved Hide resolved
scripts/exregional_nexus_emission.sh Outdated Show resolved Hide resolved
scripts/exregional_nexus_gfs_sfc.sh Outdated Show resolved Hide resolved
scripts/exregional_nexus_gfs_sfc.sh Outdated Show resolved Hide resolved
scripts/exregional_post_stat_o3.sh Outdated Show resolved Hide resolved
scripts/exregional_post_stat_pm25.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@chan-hoo Excellent work! I have a few comments related to the JREGIONAL_AQM_ICS/LBCS JJOBS, but otherwise, great job bringing in the [online-cmaq] JJOBS and EX- scripts!

jobs/JREGIONAL_AQM_ICS Outdated Show resolved Hide resolved
jobs/JREGIONAL_AQM_LBCS Outdated Show resolved Hide resolved
@MichaelLueken
Copy link
Collaborator

@chan-hoo I'm also not sure why the Python unittests are failing (none of the changes in this PR would affect the generate_FV3LAM_wflow script). Are you aware off what be causing the failure?

@chan-hoo
Copy link
Collaborator Author

@MichaelLueken @danielabdi-noaa, thanks for your comments. I'll update the scripts soon. @MichaelLueken, I don't think this PR causes the failure. PR #535 also has the same issue. @danielabdi-noaa, do you have any idea?

@danielabdi-noaa
Copy link
Collaborator

@chan-hoo @MichaelLueken It looks like generate_workflow.py is not finding the workflow generation log file for some reason. Thought that I may have introduced a bug in the linux machine file in my last PR (despite it passing unittest successfully), but same things happens before my PR too. Unit tests pass locallly on both hera and a linux machine. There is also a deprecation message from Github in the Actions tab, so I can't rule out changes on testing framework side. I will keep on investigatiing ...

Copy link
Collaborator

@danielabdi-noaa danielabdi-noaa left a comment

Choose a reason for hiding this comment

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

@chan-hoo Thanks for addressing my suggestions! I will approve now thanks!

Copy link
Collaborator

@RatkoVasic-NOAA RatkoVasic-NOAA left a comment

Choose a reason for hiding this comment

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

Looks good to me. Great job!

Copy link
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@chan-hoo While doing one more pas through the ex- scripts, I have identified two other minor issues. Two scripts have ${RUN_CMD_SERIAL}, but these aren't being used in exregional_nexus_post_split.sh and exregionaLpre_post_stat.sh. It would probably be best to remove these if they aren't being used in the scripts.

scripts/exregional_nexus_post_split.sh Outdated Show resolved Hide resolved
scripts/exregional_pre_post_stat.sh Outdated Show resolved Hide resolved
Copy link
Collaborator

@BenjaminBlake-NOAA BenjaminBlake-NOAA left a comment

Choose a reason for hiding this comment

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

My suggestions have been addressed by @danielabdi-noaa 's comments so I think it looks good now. Thanks @chan-hoo for putting this together.

Copy link
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@chan-hoo Thank you very much for addressing my concerns! Approving this work now.

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.

5 participants