ufs-community / ufs-srweather-app

UFS Short-Range Weather Application
Other
55 stars 116 forks source link

[production/AQM.v7] update workflow for AQMv7 implementation #958

Closed JianpingHuang-NOAA closed 10 months ago

JianpingHuang-NOAA commented 10 months ago

The PR is used to allow the system to use static fix dir by rocoto workflow for each run test. The purpose is to get rid of creating symbolic link of fix for each run

DESCRIPTION OF CHANGES:

The change is used to update setup.py script

Type of change

TESTS CONDUCTED:

CHECKLIST

LABELS (optional):

A Code Manager needs to add the following labels to this PR:

chan-hoo commented 10 months ago

@JianpingHuang-NOAA, does the rocoto workflow run without this temporary fix?

JianpingHuang-NOAA commented 10 months ago

@chan-hoo Yes. We need to run ~/ush/auto_A1_cp_fix_link_fix_lam.sh to create fix dir at the begging of the system set up and then never do any symbolic link to the fix dir after that. That is what we want. This is consist with the way that ECFLOW does.

chan-hoo commented 10 months ago

@JianpingHuang-NOAA, do you mean that we should run auto_A1_cp_fix_link_fix_lam.sh manually again? 1) I don't agree with this approach (manual step). Please add the command somewhere in the scripts. 2) You just remove the part for creating FIXlam, but you didn't change the following sym-link commands for this FIXlam (ln 1193). If they are not necessary either, please remove them together.

JianpingHuang-NOAA commented 10 months ago

@chan-hoo 1. I agree with you that it is not good to do copying fix files manually. But that is what we propose for the AQMv7 implementation (see our implementation instruction at https://docs.google.com/document/d/1QiK_-aQdN3qfcgD3hxyxCDu8KpMVWiDp25-itm3aTzg/edit#heading=h.30j0zll). You can add them back after the AQMv7 implementation.

  1. I tried to remove Line 1193 in setup.py file, but it failed with make_ics and make_lbs. You can check the run log file, :/lfs/h2/emc/ptmp/jianping.huang/emc.para/output/20231031 > vim make_lbcs_2023103100.id_1698759222.log. Is this what you suggest? If yes, I can not do that since it failed. The one that I used to open PR has been testing for 4 cycles for 20231030. The results are identical. Please merge as I propose. For the community use, you can add them back after the AQMv7 implementation. Thanks !
chan-hoo commented 10 months ago

@JianpingHuang-NOAA, Is the "rocoto" workflow mandatory for the AQM.v7 implementation? It doesn't make sense to me. In the previous PR for ecFlow, it screwed up the rocoto part and the rocoto option didn't work correctly. But you said it would be fine for the AQM implementation. What is the truth?

JianpingHuang-NOAA commented 10 months ago

@chan-hoo The "rocoto" workflow is not a mandatory component of the AQM.v7 implementation. The most recent workflow that you modified continues to create a symbolic link for the "fix" directory with each run. However, it appears that your workflow deletes the "fix" directory and recreates a symbolic link under the package/aqm.v7.0.0 directory each time, which is not the intended behavior. Ideally, the "fix" directory should be established during the initial system setup. Once the "fix" directory is in place, it should not be deleted, and new symbolic links should not be created.

chan-hoo commented 10 months ago

@JianpingHuang-NOAA, I agree with the concept of the fix directory (removal of the temporary change). If you make a change correctly, I'll approve this PR. However, 1) you are adding a new "hidden" step for it. 2) Your change for FIXlam is incomplete. For the "ecFlow"-related PRs, I approved them without review because it was the EMC management's decision for the AQM.v7 implementation. As you said, the rocoto option is not required by the AQM.v7 implementation. Therefore, I can't approve any incomplete change.

chan-hoo commented 10 months ago

For the record: I'll approve and merge this PR as it is now because the due date for AQM.v7 is today. I'll fix the above issue later.