[production/AQM.v7] Remove extrn_ic/lbcs and nexus_gfs_sfc

chan-hoo commented 1 year ago

DESCRIPTION OF CHANGES:

Move the get_extrn_ics, get_extrn_lbcs, and nexus_gfs_sfc tasks into their subsequent tasks make_ics, make_lbcs, and nexus_emission.
Remove the j-job and ex-scripts for the above three unnecessary tasks.

Type of change

[x] Bug fix (non-breaking change which fixes an issue)
[x] 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:

[ ] hera.intel
[ ] orion.intel
[ ] hercules.intel
[ ] cheyenne.intel
[ ] cheyenne.gnu
[ ] derecho.intel
[ ] gaea.intel
[ ] gaeac5.intel
[ ] jet.intel
[x] wcoss2.intel
[ ] NOAA Cloud (indicate which platform)
[ ] Jenkins
[ ] fundamental test suite
[ ] comprehensive tests (specify which if a subset was used)

chan-hoo commented 1 year ago

@JianpingHuang-NOAA, Please test this PR "thoroughly" unlike the previous PR. I asked you to test the previous PR with your NRT run and you said it worked well. However, the previous PR screwed up the rocoto part. I don't understand how your test was passed successfully. I had to spend some time to fix it in this PR.

chan-hoo commented 1 year ago

@JianpingHuang-NOAA, @rmontuoro, @MatthewPyle-NOAA, @HaixiaLiu-NOAA, @aerorahul, @lgannoaa, Based on the NCO standards (page 13, viii) (e.g. a forecast job writing output that is needed by a post job running in parallel), I restore this capability (running run_fcst and run_post in parallel) for AQM in this PR. It was initially designed to work for SRW as well as AQM, but I was requested to remove this capability for AQM some time ago. This caused some confusion regarding the unique_id issue. I think this capability is allowed by the NCO standards. Therefore, I think the unique_id issue in AQM will be resolved by this PR (at least in the "rocoto" perspective).

JianpingHuang-NOAA commented 1 year ago

@chan-hoo Two questions 1) Which PR do you refer ? If you refer to the latest PR that Lin opened, I was told by Lin that is for the operational implementation and we are not allowed to test it directly without any change. If you change some change for rocoto's use, that will cause a trouble to ecflow. Please check with Lin. 2) Does the PR 955 include the changes that we discussed yesterday? @lgannoaa

chan-hoo commented 1 year ago

@JianpingHuang-NOAA, understood. However, we should test this PR with rocoto first. Then, we can ask @lgannoaa to open a new PR for ecflow because we are not familiar with ecflow. Please test this PR with your NRT run. If it works, we can ask Lin for a next PR for ecflow.

lgannoaa commented 1 year ago

@JianpingHuang-NOAA, @rmontuoro, @HaixiaLiu-NOAA, @aerorahul, @lgannoaa, Based on the NCO standards (page 13, viii) (e.g. a forecast job writing output that is needed by a post job running in parallel), I restore this capability (running run_fcst and run_post in parallel) for AQM in this PR. It was initially designed to work for SRW as well as AQM, but I was requested to remove this capability for AQM some time ago. This caused some confusion regarding the unique_id issue. I think this capability is allowed by the NCO standards. Therefore, I think the unique_id issue in AQM will be resolved by this PR.

@chan-hoo please only proceed with the work that we have agreed yesterday. There is no agreement for you to modify forecast and post at this time.

chan-hoo commented 1 year ago

@JianpingHuang-NOAA, regarding your second question, yes, this PR removes get_extrn_ics/lbcs and nexus_gfs_sfc and adds these tasks into their subsequent tasks.

chan-hoo commented 1 year ago

@lgannoaa, you don't have to worry about it. it doesn't impact the ecflow run at all. I make this capability work only when WORKFLOW_MANAGER="rocoto".

JianpingHuang-NOAA commented 1 year ago

@chan-hoo Ye, I did test Lin's PR by modifying the following 5 ex-scripts on my local test version on WCOSS2 (/lfs/h2/emc/physics/noscrub/jianping.huang/nwdev/packages/aqm.v7.0.86a) 1) ~/script/ exregional_aqm_ics.sh_rocoto exregional_make_lbcs.sh_rocoto exregional_run_post.sh_rocoto* exregional_make_ics.sh_rocoto exregional_nexus_emission.sh_rocoto

2) ~/ush/load_modules_run_task.sh

but I did not commit these changes into the Lin's branch to avoid any trouble. This may cause some confusion to you.

chan-hoo commented 1 year ago

@JianpingHuang-NOAA, totally understood now. I am sorry for my misunderstanding regarding the previous PR. Please test this PR with your NRT run.

chan-hoo commented 1 year ago

@JianpingHuang-NOAA, when you test this PR, please don't forget to update your config.yaml as follows:

workflow_switches:
  RUN_TASK_GET_EXTRN_ICS: false
  RUN_TASK_GET_EXTRN_LBCS: false
  RUN_TASK_NEXUS_GFS_SFC: false

rmontuoro commented 1 year ago

@lgannoaa, you don't have to worry about it. it doesn't impact the ecflow run at all. I make this capability work only when WORKFLOW_MANAGER="rocoto".

@chan-hoo, @JianpingHuang-NOAA, and @lgannoaa - Please limit the code changes to what discussed yesterday. Changes to the forecast and post jobs are not required. Any change at this stage (even if only for rocoto) introduces additional risk and may have unintended consequences.

chan-hoo commented 1 year ago

@rmontuoro, the change for run_post has been removed.

JianpingHuang-NOAA commented 1 year ago

@chan-hoo I completed the test with your feature branch rm_extrn. The test results can be found on Dogwood at

/lfs/h2/emc/ptmp/jianping.huang/emc.para/com/aqm/v7.0/c86b.20231026/00

All the grib2 files are identical to the real-time test results with the old package before you made the changes. (/lfs/h2/emc/ptmp/jianping.huang/emc.para/com/aqm/v7.0/aqm.20231026/00)

So, the test is completed. Please go ahead to merge PR 995 into the branch production/AQM.v7 now. Thanks !

chan-hoo commented 1 year ago

@BenjaminBlake-NOAA, could you please approve this PR? Since I opened this PR, you are the only person who can approve this PR. :) :)

rmontuoro commented 1 year ago

@rmontuoro, the change for run_post has been removed.

Thank you, @chan-hoo. I appreciate your prompt attention and contributions to this update.

JianpingHuang-NOAA commented 1 year ago

@BenjaminBlake-NOAA, Can you review and approve the PR? I am waiting for your response. Thanks, !

chan-hoo commented 1 year ago

@BenjaminBlake-NOAA, due to the change in the previous PR for ecflow, the 'rocoto' run doesn't work any more. The change in the run_post script makes both ecflow and rocoto run correctly.

chan-hoo commented 1 year ago

@BenjaminBlake-NOAA, Thank you so much! @JianpingHuang-NOAA @rmontuoro @lgannoaa, I'll merge this PR.

JianpingHuang-NOAA commented 1 year ago

@chan-hoo Great. Thanks !

ufs-community / ufs-srweather-app