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

Turn on HAFS RTs on WCOSS2; AQM radians to degrees; NRL Neptune model 32-bit physics support #1227

Merged

Conversation

BinLiu-NOAA
Copy link
Contributor

@BinLiu-NOAA BinLiu-NOAA commented May 20, 2022

PR Checklist

  • This PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.

  • This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR

  • An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
    are specified below.

  • Results for one or more of the regression tests change and the reasons for the changes are understood and explained below.

    • Need at least create new baseline for the HAFS RTs on WCOSS2
    • Need hafs_regional_storm_following_1nest_atm on hera.intel
  • New or updated input data is required by this PR. If checked, please work with the code managers to update input data sets on all platforms.

Instructions: All subsequent sections of text should be filled in as appropriate.

The information provided below allows the code managers to understand the changes relevant to this PR, whether those changes are in the ufs-weather-model repository or in a subcomponent repository. Ufs-weather-model code managers will use the information provided to add any applicable labels, assign reviewers and place it in the Commit Queue. Once the PR is in the Commit Queue, it is the PR owner's responsibility to keep the PR up-to-date with the develop branch of ufs-weather-model.

Description

Provide a detailed description of what this PR does. What bug does it fix, or what feature does it add? Is a change of answers expected from this PR? Are any library updates included in this PR (modulefiles etc.)?

  • Turn on HAFS RTs on WCOSS2 and switch to use the netcdf (instead of netcdf_parallel) write_grid_component option for HAFS RTs.
    • When using the netcdf_parallel write grid component option, the forecast will hang on WCOSS2 during writing out netcdf history output files. There were similar issues on other platforms occasionally as well.
  • Update to use fms/2022.01 on wcoss_cray.
  • This PR combines PR AQM radians to degrees #1212
    • To fix an offset of PM2.5 peaks a coordinate change from radians to degrees was made.
    • There are no changes to results as there are no RT's for AQM yet.
    • Recommend bringing this PR in before AQM RT addition or along with it.
  • This PR also combines PR NRL Neptune model 32-bit physics support #1206
    • Adds ccpp-physics changes needed to support 32-bit physics on Neptune. This does not add 32-bit physics to FV3, which requires quite a lot more changes in several repositories.

Issue(s) addressed

Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues must always be created before starting work on a PR branch!)

Testing

Tests conducted on WCOSS2 run through all HAFS RTs.

How were these changes tested? What compilers / HPCs was it tested with? Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) Have regression tests and unit tests (utests) been run? On which platforms and with which compilers? (Note that unit tests can only be run on tier-1 platforms)

Dependencies

If testing this branch requires non-default branches in other repositories, list them. Those branches should have matching names (ideally).

@BinLiu-NOAA BinLiu-NOAA added enhancement New feature or request Baseline Updates Current baselines will be updated. labels May 20, 2022
tests/rt.conf Outdated Show resolved Hide resolved
@BrianCurtis-NOAA
Copy link
Collaborator

@BinLiu-NOAA Please add a WCOSS2 item in the ufs-weather-model/.github/pull_request_template.md

@BinLiu-NOAA
Copy link
Contributor Author

@BinLiu-NOAA Please add a WCOSS2 item in the ufs-weather-model/.github/pull_request_template.md

@BrianCurtis-NOAA, done. Thanks!

@DeniseWorthen DeniseWorthen linked an issue May 25, 2022 that may be closed by this pull request
@BrianCurtis-NOAA
Copy link
Collaborator

@BinLiu-NOAA Its now your turn at the top of the commit queue. Please update .gitmodules with #1206 (fv3atm) changes and #1212 (AQM) changes.

@BrianCurtis-NOAA
Copy link
Collaborator

@BinLiu-NOAA With the error of variables being different in the hafs_regional_.._gfdl test, you're OK with regenerating that one test, correct?

@BinLiu-NOAA
Copy link
Contributor Author

@BinLiu-NOAA With the error of variables being different in the hafs_regional_.._gfdl test, you're OK with regenerating that one test, correct?

Yes, please help to go ahead regenerating baseline for this test. There is a change from netcdf_parallel to netcdf output option, which might have affected the netcdf output files. Thanks!

@junwang-noaa
Copy link
Collaborator

@BinLiu-NOAA Would you please check the differences between the two files(netcdf_parallel and netcdf) to confirm the data fields are correct? Thanks

@BinLiu-NOAA
Copy link
Contributor Author

BinLiu-NOAA commented May 26, 2022

@BinLiu-NOAA Would you please check the differences between the two files(netcdf_parallel and netcdf) to confirm the data fields are correct? Thanks

@junwang-noaa and @BrianCurtis-NOAA, Sure, doing that. Actually found an interesting issue when comparing the new generated baseline:
ncdump -h /gpfs/hps3/emc/nems/noscrub/emc.nemspara/RT/NEMSfv3gfs/develop-20220519/hafs_regional_atm_thompson_gfdlsf/atmf006.nc
against the previous version baseline on wcoss_cray:
ncdump -h /gpfs/hps3/emc/nems/noscrub/emc.nemspara/RT/NEMSfv3gfs/develop-20220516/hafs_regional_atm_thompson_gfdlsf/atmf006.nc

The new baseline got the following three variables missing, for which I don't know why. Other than that the other variables actually got identical values. I believe the only thing this PR changed for this hafs_regional_atm_thompson_gfdlsf test is using netcdf instead of netcdf_parallel output. Also, another special thing for this test is it uses the "cubed_sphere" native grid output option.

    char time_iso(time, nchars) ;
            time_iso:long_name = "valid time" ;
            time_iso:description = "ISO 8601 datetime string" ;
    double lon(grid_yt, grid_xt) ;
            lon:long_name = "T-cell longitude" ;
            lon:units = "degrees_E" ;
    double lat(grid_yt, grid_xt) ;
            lat:long_name = "T-cell latitude" ;
            lat:units = "degrees_N" ;

@SamuelTrahanNOAA
Copy link
Collaborator

SamuelTrahanNOAA commented May 26, 2022

The fv3gfs_diag_output never sends those three variables directly, and they're not in the diag_table. Maybe the diag manager is supposed to add them automatically? If so, this is a bug in FMS, beyond Bin's control.

Edit: They're not in any diag tables in tests/parm/diag_table

@DusanJovic-NOAA
Copy link
Collaborator

@BinLiu-NOAA Would you please check the differences between the two files(netcdf_parallel and netcdf) to confirm the data fields are correct? Thanks

@junwang-noaa and @BrianCurtis-NOAA, Sure, doing that. Actually found an interesting issue when comparing the new generated baseline: ncdump -h /gpfs/hps3/emc/nems/noscrub/emc.nemspara/RT/NEMSfv3gfs/develop-20220519/hafs_regional_atm_thompson_gfdlsf/atmf006.nc against the previous version baseline on wcoss_cray: ncdump -h /gpfs/hps3/emc/nems/noscrub/emc.nemspara/RT/NEMSfv3gfs/develop-20220516/hafs_regional_atm_thompson_gfdlsf/atmf006.nc

The new baseline got the following three variables missing, for which I don't know why. Other than that the other variables actually got identical values. I believe the only thing this PR changed for this hafs_regional_atm_thompson_gfdlsf test is using netcdf instead of netcdf_parallel output. Also, another special thing for this test is it uses the "cubed_sphere" native grid output option.

    char time_iso(time, nchars) ;
            time_iso:long_name = "valid time" ;
            time_iso:description = "ISO 8601 datetime string" ;
    double lon(grid_yt, grid_xt) ;
            lon:long_name = "T-cell longitude" ;
            lon:units = "degrees_E" ;
    double lat(grid_yt, grid_xt) ;
            lat:long_name = "T-cell latitude" ;
            lat:units = "degrees_N" ;

Different output routines are used for 'cubed_sphere_grid' output_grid with 'netcdf' (ESMFproto_FieldBundleWrite) than for 'netcdf_parallel' (write_netcdf).

@junwang-noaa
Copy link
Collaborator

@BinLiu-NOAA Thanks for checking. I think the valid time was only added to non-native grid history file output. We are using ESMF calls to write out native grid output. This is a different issue not related to this PR. @DusanJovic-NOAA FYI.

@BinLiu-NOAA
Copy link
Contributor Author

Different output routines are used for 'cubed_sphere_grid' output_grid with 'netcdf' (ESMFproto_FieldBundleWrite) than for 'netcdf_parallel' (write_netcdf).

Thanks for this information, @DusanJovic-NOAA! With that, @junwang-noaa and @BrianCurtis-NOAA, I would suggest just moving forward with the new re-generated baseline for this specific hafs_regional_atm_thompson_gfdlsf test. And let this issue/inconsistency be fixed/addressed in a future PR. Thanks!

@BrianCurtis-NOAA
Copy link
Collaborator

Automated RT Failure Notification
Machine: gaea
Compiler: intel
Job: RT
[RT] Repo location: /lustre/f2/pdata/ncep/emc.nemspara/autort/pr/942703315/20220526113015/ufs-weather-model
[RT] Error: Test hafs_regional_atm_thompson_gfdlsf 095 failed in check_result failed
[RT] Error: Test hafs_regional_atm_thompson_gfdlsf 095 failed in run_test failed
Please make changes and add the following label back: gaea-intel-RT

@junwang-noaa
Copy link
Collaborator

@BrianCurtis-NOAA Are you going to rerun the hafs_regional_atm_thompson_gfdlsf test?

@BrianCurtis-NOAA
Copy link
Collaborator

BrianCurtis-NOAA commented May 26, 2022

@BrianCurtis-NOAA Are you going to rerun the hafs_regional_atm_thompson_gfdlsf test?

Yes I am. @DeniseWorthen did a compare and found new/previous baseline to be the same even though it showed as failed. I started the re-run before 3 but it's stuck in the Gaea queue still. Apologies it's going slow today.

@BrianCurtis-NOAA
Copy link
Collaborator

BrianCurtis-NOAA commented May 26, 2022

@junwang-noaa ready for merges for ccpp-physics and AQM

@BrianCurtis-NOAA
Copy link
Collaborator

@BinLiu-NOAA Please update .gitmodules and AQM 73da18b and FV3 515a39d hashes

@BrianCurtis-NOAA BrianCurtis-NOAA merged commit 71c752a into ufs-community:develop May 26, 2022
@BinLiu-NOAA BinLiu-NOAA deleted the feature/hafs_wcoss2_rts branch May 26, 2022 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated. enhancement New feature or request
Projects
None yet
8 participants