Update ecflow package for wcoss2 GFS transition #555

lgannoaa · 2022-01-07T18:18:57Z

This PR:

No need to review obsproc related files and link under ecflow. NCO will work on new ops version.
Updates the 4 suite definition files prodXX.def; XX=00,06,12,18
Includes updates to facilitate developer testing by specifying developer values for variables such as QUEUE,
QUEUE_TRANSFER, QUEUE_SHARED and COM
Modify module position in each ecflow job using the same position from
$HOMEgfs/modulefiles/module_base.wcoss2.lua
Adding envir-p1.h for wcoss2 environment
Include head.h for reference developer setting
Change in ecflow script:
- Updates the ecflow scripts to consistent with GFS module position
- Using wcoss2 environment
- Using PBS for WCOSS2
- Using updated module
- Move enkf out of gdas and rename it to enkfgdas.
  Include all ecflow definition files job name
  Include all ecflow scripts name and job/log name
- Move "model=gfs" to the top on each job except all jobs under obsproc.
  obsproc will no longer be part of GFS. Therefore leave it without change for testing purpose.
- Remove the source of model_ver from each ecflow script except all jobs under obsproc.
  obsproc will no longer be part of GFS. Therefore leave it without change for testing purpose.
  This work is part of WCOSS2 Migration and Porting #398

WCOSS2 Migration and Porting NOAA-EMC#398

aerorahul

I have reviewed upto the file:
ecf/scripts/gfs/atmos/post_processing/bufr_sounding/jgfs_atmos_postsnd.ecf
But not head.h as it is very complicated.

Also PR #553 has been merged, so please rebase this PR based on current feature/ops-wcoss2

Please see comments.

ecf/scripts/gdas/atmos/obsproc/dump/jgdas_atmos_dump_alert.ecf

aerorahul · 2022-01-07T19:41:07Z

ecf/include/envir-p1.h

+#module load prod_envir/2.0.4 prod_util/2.0.9 envvar/1.0
+echo debug


Please remove commented and unnecessary echo

ecf/scripts/gdas/atmos/gempak/jgdas_atmos_gempak.ecf

aerorahul · 2022-01-07T19:45:03Z

ecf/scripts/gdas/atmos/init/jgdas_atmos_gldas.ecf

@@ -43,6 +37,7 @@ module list
 #############################################################
 export cyc=%CYC%
 export cycle=t%CYC%z
+export CDATE=$PDY$cyc


This variable is set in envir-p1.h. It does not need to be set here.

aerorahul · 2022-01-07T19:50:49Z

ecf/scripts/gfs/atmos/gempak/jgfs_atmos_gempak_ncdc_upapgif.ecf

+module load gempak/${gempak_ver}
+module load libjpeg/${libjpeg_ver}
+module load grib_util/${grib_util_ver}
+module load imagemagick/${imagemagick_ver}


Where is ${imagemagic_ver} coming from? I don't see it here

This job need module imagemagic

Yes. I understand that. But what is the value of imagemagik_ver and where is it set?

It need to be set in run.ver

aerorahul · 2022-01-07T19:53:40Z

ecf/scripts/gfs/atmos/post/jgfs_atmos_post_f000.ecf

+#PBS -q %QUEUE%
+#PBS -A %PROJ%-%PROJENVIR%
+#PBS -l walltime=00:20:00
+#PBS -l place=vscatter,select=10


@WenMeng-NOAA On WCOSS_Dell, this job (and the other jgfs_atmos_post_fXXX jobs), they take 4 Nodes totaling to 112 tasks. See lines 17,18 on the left.
Am I reading this right that this job (and other jgfs_atmos_post_fXXX jobs) are requesting 10 nodes totaling to 1280 tasks for a single forecast hour.

ecf/scripts/gfs/atmos/post/jgfs_atmos_post_manager.ecf

aerorahul · 2022-01-07T20:08:00Z

ecf/scripts/gfs/atmos/post/jgfs_atmos_post_manager.ecf

+#PBS -q %QUEUE%
+#PBS -A %PROJ%-%PROJENVIR%
+#PBS -l walltime=04:00:00
+#PBS -l place=vscatter,select=1:ncpus=128


Surely this job only needs 1 task as it only sleeps

WalterKolczynski-NOAA

Reviewed from jgfs_atmos_post_anl.ecf to the end.

WalterKolczynski-NOAA · 2022-01-11T15:17:12Z

ecf/versions/obsproc.ver

+export dictionaries_ver="v3.4.0"
+export omb_ver="v4.4"
+
+## main "model" software


All these commented out lines should be removed.

WalterKolczynski-NOAA · 2022-01-11T15:17:40Z

ecf/versions/gfs.ver

+export radarl2_ver=v1.2
+
+#### NCO requested testing location is v16.2
+#### export gfs_ver="v16.2.0"


Removed unused line

ecf/scripts/gfs/atmos/post/jgfs_atmos_post_manager.ecf

aerorahul · 2022-01-11T16:39:52Z

This PR has been partially reviewed.
The changes are too many to be made at this time for a thorough cleanup and review and retest.
Please merge as it only affects ecflow and only a single developer can run this setup in its current form due to hardwired paths, username, and ECF_PORT number in the ecflow files such as .def, head.h, model_ver.h, envir_p1.h.

aerorahul · 2022-01-11T16:16:12Z

ecf/include/envir-p1.h

+#### Developer overwrite
+    export SIPHONROOT=${UTILROOT}/fakedbn
+####
+else
+    export SIPHONROOT=${UTILROOT}/fakedbn
+fi


This is developer only. Not used by NCO and thus will not be addressed by NCO.
Please resolve the if-else logic as it is executed regardless of the condition.

aerorahul · 2022-01-11T16:17:00Z

ecf/include/envir-p1.h

+
+export DBNROOT=$SIPHONROOT
+
+####if [[ ! " prod para test " =~ " ${envir} " && " ops.prod ops.para " =~ " $(whoami) " ]]; then err_exit "ENVIR must be prod, para, or test [envir-p1.h]"; fi


Why is this commented out? If it is unused, what purpose does it serve?
Either put a comment on this explaining the purpose or remove.

ecf/include/head.h

ecf/include/envir-p1.h

aerorahul · 2022-01-11T16:25:02Z

ecf/include/envir-p1.h

+export DBNROOT=$SIPHONROOT
+
+####if [[ ! " prod para test " =~ " ${envir} " && " ops.prod ops.para " =~ " $(whoami) " ]]; then err_exit "ENVIR must be prod, para, or test [envir-p1.h]"; fi
+export ECF_PORT=34326


Please remove this line. This variable is set when ecflow module is loaded. ecflow module is loaded in head.h. This variable is unique to the user.

The job tested without this line sometime failed silently. There is a good reason to include this in here.
As discussed, this file will be used only for reference. I prefer to keep it.

Sometimes failure is a symptom of the system instability. I would rather encounter this failure sometimes, than hard-wire some users personal ECF_PORT information. I suggest removal.

aerorahul · 2022-01-11T20:43:31Z

ecf/scripts/gdas/atmos/analysis/jgdas_atmos_analysis.ecf


 module list

 #############################################################
 # WCOSS environment settings
 #############################################################
-export KMP_AFFINITY=scatter
 export OMP_NUM_THREADS=$threads
 export FORT_BUFFERED=true
 export OMP_NUM_THREADS_CY=28


Is OMP_NUM_THREADS_CY=28 appropriate for WCOSS2? config.resources.nco.static is setting this to npe_node_max. 28 is the value for WCOSS_Dell_P3 and was hard-wired here.

The same is the case in jgfs_atmos_analysis.ecf.

Set to 128

Are you sure, the value from this variable set here OMP_NUM_THREADS_CY is being used?
I see https://github.com/NOAA-EMC/GSI/blob/a9561f4219e5a69ac1af250081ae027084c40c3b/scripts/exglobal_atmos_analysis.sh#L1017 setting the value based on NTHREADS_CYCLE right before entering CYCLESH

aerorahul · 2022-01-11T20:57:11Z

ecf/scripts/gdas/atmos/analysis/jgdas_atmos_analysis.ecf


 module list

 #############################################################
 # WCOSS environment settings
 #############################################################
-export KMP_AFFINITY=scatter
 export OMP_NUM_THREADS=$threads


Where is the value for $threads coming from? It was previously set on line 20 above and has been removed in this PR.

…re/ecflow-ops-ecf2

- Move enkf out of gdas and rename it to enkfgdas. Include all ecflow definition files job name Include all ecflow scripts name and job/log name - Move "model=gfs" to the top on each job except all jobs under obsproc. obsproc will no longer be part of GFS. Therefore leave it without change for testing purpose. - Remove the source of model_ver from each ecflow script except all jobs under obsproc. obsproc will no longer be part of GFS. Therefore leave it without change for testing purpose.

Update analysis ecflow script to use 128 for wcoss2 Remove extra CDATE

aerorahul · 2022-01-11T22:27:17Z

ecf/include/envir-p1.h

+if [[ "$envir" == prod && "$SENDDBN" == YES ]]; then
+    export eval=%EVAL:NO%
+    if [ $eval == YES ]; then 
+      export SIPHONROOT=${UTILROOT}/para_dbn
+    else 
+      export SIPHONROOT=/lfs/h1/ops/prod/dbnet_siphon
+    fi
+    export SIPHONROOT=${UTILROOT}/fakedbn
+else
+    export SIPHONROOT=${UTILROOT}/fakedbn
+fi


Based on the logic here, regardless of the conditions, the result is:
export SIPHONROOT=${UTILROOT}/fakedbn
Also, a variable eval is being exported for use somewhere down when envir == prod and SENDDBN is YES.
Is that the desired outcome?

The developer will always use fakedbn to test. eval is ok here

What does eval do? Is it used anywhere other than the if-test?
If the developer will always use fakeddbn, then the first if test "$envir" == prod is not necessary (and this whole block just boiled down to this):

export SIPHONROOT=${UTILROOT}/fakedbn

aerorahul

Please see more comments.

aerorahul · 2022-01-11T23:07:18Z

ecf/defs/prod00.def

@@ -1,6 +1,6 @@
 extern /prod18/gdas/jgdas_forecast
 extern /prod18/gdas/atmos/post
-extern /prod18/gdas/enkf/post
+extern /prod18/enkfgdas/post


These are new changes added in these .def files that were agreed to be added in a follow-up PR in the meeting this AM. This PR is already big to add any additional changes.

ecf/include/envir-p1.h

aerorahul · 2022-01-11T23:09:19Z

ecf/include/head.h

+  module load ecflow/$ecflow_ver
+  echo "ecflow module location: $(module display ecflow |& head -2 | tail -1 | sed 's/:$//')"
+  export ECF_ROOT=/apps/ops/prod/nco/core/ecflow.v5.6.0.7
+  export ECF_PORT=34326


Please remove user specific PORT information. This value is set in the ecflow module for the user and is unique to the user.

aerorahul · 2022-01-11T23:10:05Z

ecf/include/head.h

+  echo "Running module load ecflow/$ecflow_ver"
+  module load ecflow/$ecflow_ver
+  echo "ecflow module location: $(module display ecflow |& head -2 | tail -1 | sed 's/:$//')"
+  export ECF_ROOT=/apps/ops/prod/nco/core/ecflow.v5.6.0.7


There are dual uses of the variable ecflow_ver and 5.6.0.7 for ecflow version. Which one is it? Please be consistent in the use of the version.

aerorahul · 2022-01-11T23:11:28Z

ecf/include/head.h

+  echo "Running module load ecflow/$ecflow_ver"
+  module load ecflow/$ecflow_ver
+  echo "ecflow module location: $(module display ecflow |& head -2 | tail -1 | sed 's/:$//')"
+  export ECF_ROOT=/apps/ops/prod/nco/core/ecflow.v5.6.0.7


This variable is set by the ecflow module. It should not be overwritten. Load the appropriate module instead.
Please remove.

aerorahul · 2022-01-11T23:12:43Z

ecf/include/head.h

+  export ECF_ROOT=/apps/ops/prod/nco/core/ecflow.v5.6.0.7
+  . ${ECF_ROOT}/versions/run.ver


I am failing to understand what this is doing. Why export? and why loading prod_util in head.h

aerorahul · 2022-01-11T23:14:13Z

ecf/include/head.h

+  export ECF_ROOT=/apps/ops/prod/nco/core/ecflow.v5.6.0.7
+  export ECF_PORT=34326
+  export ECF_HOST=ddecflow02
+  export ECF_INCLUDE=/lfs/h2/emc/global/noscrub/$USER/git/feature-ops-wcoss2/ecf/include


This is hardwired to a specific branch which will not exist as soon as it is merged and deleted.
I am failing to understand the need for this entire section from 40-47. It seems to be tailored for a specific branch and specific location.

aerorahul · 2022-01-11T23:14:48Z

ecf/scripts/enkfgdas/analysis/create/jenkfgdas_diag.ecf

@@ -0,0 +1,51 @@
+#PBS -S /bin/bash


These EnKF jobs are new and we agreed to add them in a followup PR to avoid further bloating this current PR.

KateFriedman-NOAA · 2022-01-12T16:20:30Z

Forcibly submitting this PR due to need.

ecflow package for wcoss2 GFS transition

68651b3

WCOSS2 Migration and Porting NOAA-EMC#398

aerorahul requested changes Jan 7, 2022

View reviewed changes

WalterKolczynski-NOAA requested changes Jan 11, 2022

View reviewed changes

aerorahul reviewed Jan 11, 2022

View reviewed changes

lgannoaa added 5 commits January 11, 2022 21:10

Merge remote-tracking branch 'upstream/feature/ops-wcoss2' into featu…

c5a3375

…re/ecflow-ops-ecf2

Update enkf structure changes in ecflow definition files

139c33c

Remove remark from envir-p1.h and head.h

b2a57df

Update analysis ecflow script to use 128 for wcoss2 Remove extra CDATE

Remove ecflow post assignment in envir-p1.h

a78d6c7

aerorahul reviewed Jan 11, 2022

View reviewed changes

aerorahul requested changes Jan 11, 2022

View reviewed changes

KateFriedman-NOAA merged commit 82aad0c into NOAA-EMC:feature/ops-wcoss2 Jan 12, 2022

aerorahul mentioned this pull request Jan 13, 2022

Remove duplicate ecf scripts that are just copies of f000 in all cases. #574

Merged

		#module load prod_envir/2.0.4 prod_util/2.0.9 envvar/1.0
		echo debug


		export DBNROOT=$SIPHONROOT

		####if [[ ! " prod para test " =~ " ${envir} " && " ops.prod ops.para " =~ " $(whoami) " ]]; then err_exit "ENVIR must be prod, para, or test [envir-p1.h]"; fi

		export ECF_ROOT=/apps/ops/prod/nco/core/ecflow.v5.6.0.7
		. ${ECF_ROOT}/versions/run.ver

Update ecflow package for wcoss2 GFS transition #555

Update ecflow package for wcoss2 GFS transition #555

Conversation

lgannoaa commented Jan 7, 2022 • edited Loading

aerorahul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WalterKolczynski-NOAA left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aerorahul commented Jan 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aerorahul Jan 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aerorahul left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KateFriedman-NOAA commented Jan 12, 2022

lgannoaa commented Jan 7, 2022 •

edited

Loading

aerorahul Jan 11, 2022 •

edited

Loading