Move PYTHONPATH configuration from load_modules.sh to J-Jobs that call Python scripts#4564
Conversation
Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
|
@copilot the issue explicitly says PYTHONPATH, when set in the J-Job, should only point to USHglobal paths (i.e. not to sorc) so we need to ensure that those paths are linked to ush/python as part of the link_workflow script. |
- Modified link_workflow.sh to create symlink from sorc/wxflow/src/wxflow to ush/python/wxflow
- Updated 44 J-Jobs to remove sorc/wxflow/src from PYTHONPATH
- J-Jobs now only reference ${HOMEgfs}/ush/python in PYTHONPATH
- This satisfies acceptance criterion #4: PYTHONPATH only points to USHglobal paths
Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Fixed in commit aba791f. The |
|
@copilot do all 46 j-jobs need the PYTHONPATH updated? Should it only be scripts that call python ex scripts? |
- Removed PYTHONPATH from 6 J-Jobs that only use compath.py (a prod_util utility) - These jobs don't call Python ex scripts and don't need PYTHONPATH: * JGDAS_ATMOS_GEMPAK_META_NCDC * JGFS_ATMOS_CYCLONE_GENESIS * JGFS_ATMOS_CYCLONE_TRACKER * JGFS_ATMOS_FSU_GENESIS * JGFS_ATMOS_GEMPAK_META * JGLOBAL_ATMOS_CHGRES_GEN_CONTROL (calls .sh script, not .py) - Result: 40 J-Jobs with PYTHONPATH, all calling Python execution scripts Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Correct - PYTHONPATH should only be in scripts that call Python execution scripts. Fixed in commit 2b82f24. Removed PYTHONPATH from 6 J-Jobs that only use
Result: 40 J-Jobs have PYTHONPATH (all call Python ex scripts), 6 removed (don't need it). |
| exit 1 | ||
| fi | ||
|
|
||
| # Set up the PYTHONPATH to include wxflow from HOMEgfs |
There was a problem hiding this comment.
@copilot also add ush/python to PYTHONPATH here when this is removed, thanks
There was a problem hiding this comment.
Done in commit 2ae1a08. Added PYTHONPATH="${HOMEgfs}/ush/python${PYTHONPATH:+:${PYTHONPATH}}" to gw_setup.sh so workflow setup scripts have access to pygfs and wxflow (via symlink in ush/python).
There was a problem hiding this comment.
@copilot we actually shouldn't change this file because the wxflow directory will have not yet been linked before anything is built and this is needed to build. So please revert all changes to this file
There was a problem hiding this comment.
There was a problem hiding this comment.
@copilot you misunderstood me, I want this file reverted to develop. We should not be changing it at all.
There was a problem hiding this comment.
Reverted to develop in commit 5c572ec. The file is now completely unchanged from its original state, including the PYTHONPATH configuration that points to sorc/wxflow/src. This file is no longer modified as part of this PR.
- Added PYTHONPATH="${HOMEgfs}/ush/python${PYTHONPATH:+:${PYTHONPATH}}" to gw_setup.sh
- This ensures workflow setup scripts have access to pygfs and wxflow (via symlink)
- Addresses review comment to add ush/python when removing sorc/wxflow/src reference
Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
- Reverted all PYTHONPATH changes to gw_setup.sh - The wxflow symlink doesn't exist during workflow setup/build phase - gw_setup.sh is used before link_workflow.sh creates the symlink - File now has no PYTHONPATH configuration (same as ae0ce17) Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
CoryMartin-NOAA
requested review from
CatherineThomas-NOAA,
ChristopherHill-NOAA,
DavidNew-NOAA,
JesseMeng-NOAA,
TravisElless-NOAA,
WenMeng-NOAA and
bbakernoaa
as code owners
|
Some of my tests on Ursa are looking good after one full cycle so I'm going to mark this ready for review. Will post details tomorrow AM and then we can use CI for more thorough testing. |
|
Approved pending successful tests on Ursa. |
|
Running the -G and -C tests on Ursa worked overnight. @DavidHuber-NOAA feel free to kick off testing on another machine but I think this is ready to go. |
|
Launching on C6. |
emcbot
added
CI-Gaeac6-Ready
|
C96C48mx500_S2SW_cyc_gfs FAILED on Gaeac6 (pipeline ID: 9221) In directory: Error Log Files: View Error Logs: (gdas_arch_tar_gdas_restartb.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
emcbot
added
CI-Gaeac6-Failed
|
All tests passed on C6 except for archiving during the |
DavidHuber-NOAA
added
CI-Gaeac6-Passed (cm)
|
Woohoo! I love a good AI PR that works with minimal intervention |
…l Python scripts (NOAA-EMC#4564) Move PYTHONPATH configuration from load_modules.sh to J-Jobs that call Python scripts
✅ Move PYTHONPATH configuration from load_modules.sh to J-Jobs that call Python scripts
Summary
Successfully moved PYTHONPATH configuration from centralized setup scripts to individual J-Job files. Each job that calls Python execution scripts sets only the paths it needs, with all paths pointing to USHglobal (ush/python) as required. The workflow setup script (
gw_setup.sh) is unchanged from develop.Acceptance Criteria Status
load_modules.shno longer sets PYTHONPATH explicitly (except UFSDA case for gdas.cd)ush/python) ✨Changes Made (50 files)
Setup Scripts (2):
dev/ush/load_modules.sh: Removed generic PYTHONPATH (lines 219-226)sorc/link_workflow.sh: Added wxflow symlink to ush/python (lines 98-105)Unchanged:
dev/ush/gw_setup.sh: No changes - remains in original develop state with PYTHONPATH set to sorc/wxflow/srcJ-Jobs Updated (48 total, 42 with PYTHONPATH):
${HOMEgfs}/ush/pythonin PYTHONPATHcompath.py(prod_util utility), don't need PYTHONPATHImplementation Pattern
Jobs without PYTHONPATH (don't call Python execution scripts):
Key Change: wxflow Linking
The
link_workflow.shscript creates a symlink:ush/python/wxflow -> sorc/wxflow/src/wxflowThis allows J-Jobs to reference only
ush/pythonwhile still accessing wxflow functionality.Important Note:
gw_setup.shis unchanged from develop and retains its original PYTHONPATH configuration pointing tosorc/wxflow/src. This file is not modified as part of this PR.Validation
sorc/wxflow/srcin J-JobsTesting Requirements
Runtime testing on HPC platforms (HERA, HERCULES, WCOSS2, Ursa, etc.) is needed to verify:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.