Skip to content

[GCAFSv1] Inherit HOMEgcafs in ecf scripts and set HOMEglobal#4666

Merged
CoryMartin-NOAA merged 7 commits intodev/gcafs.v1from
copilot/dev-gcafs-v1-inherit-homegcafs
Mar 20, 2026
Merged

[GCAFSv1] Inherit HOMEgcafs in ecf scripts and set HOMEglobal#4666
CoryMartin-NOAA merged 7 commits intodev/gcafs.v1from
copilot/dev-gcafs-v1-inherit-homegcafs

Conversation

Copy link
Contributor

Copilot AI commented Mar 17, 2026

Description

All non-symlink .ecf scripts under ecf/scripts/gcafs/ and ecf/scripts/gcdas/ did not expose HOMEglobal, causing any job scripts that reference ${HOMEglobal} to fail.

One change applied to the 18 non-symlink scripts:

  • export HOMEglobal=${HOMEgcafs} inserted after export EXPDIR=%EXPDIR%, grouping it with the other exports — head.h sets HOMEgcafs via eval "export HOME${model}=$modelhome"; this aliases it to HOMEglobal for compatibility with shared JGLOBAL_* job scripts

Resulting export block in every modified gcafs/gcdas script:

model=gcafs
%include <head.h>
%include <envir-p1.h>

export NET=%NET:gcafs%
export RUN=%RUN%
...
export cyc=%CYC%
export cycle=t%CYC%z
export EXPDIR=%EXPDIR%
export HOMEglobal=${HOMEgcafs}

Note: files that are symbolic links (e.g. jgcafs_atmos_post_f*.ecf and jgcdas_atmos_post_*.ecf) are left unchanged as they inherit content from their symlink targets (jgcafs_atmos_post_master.ecf and jgcdas_atmos_post_master.ecf respectively), which are themselves modified.

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this change expected to change outputs? NO
    • GFS
    • GEFS
    • SFS
    • GCAFS
  • Is this a breaking change? NO
  • Does this change require a documentation update? NO
  • Does this change require an update to any of the following submodules? NO
    • EMC verif-global
    • GDAS
    • GFS-utils
    • GSI
    • GSI-monitor
    • GSI-utils
    • UFS-utils
    • UFS-weather-model
    • wxflow

How has this been tested?

Not yet tested on a live system.

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary
Original prompt

This section details on the original issue you should resolve

<issue_title>Inherit HOMEgcafs in the ecf scripts and set HOMEglobal to it</issue_title>
<issue_description>### What new functionality do you need?

Basically #4553 but for the dev/gcafs branch

Acceptance Criteria

No response

Suggest a solution (optional)

No response</issue_description>

<agent_instructions>For every script recursively in ecf/scripts/gcafs and ecf/scripts/gcdas please add lines like below:

export model=gcafs
%include <head.h>
%include <envir-p1.h>

set -x

export NET=%NET:gcafs%
export RUN=%RUN%
export HOMEglobal=${HOMEgcafs}

Please start with the dev/gcafs.v1 branch and open a PR into that branch, not into develop</agent_instructions>

Comments on the Issue (you are @copilot in this section)


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Copilot AI changed the title [WIP] Inherit HOMEgcafs in the ecf scripts and set HOMEglobal Inherit HOMEgcafs in ecf scripts and set HOMEglobal Mar 17, 2026
Copilot AI requested a review from CoryMartin-NOAA March 17, 2026 18:41
@CoryMartin-NOAA
Copy link
Contributor

@copilot please revert the changes to the files that were previously just symbolic links

Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 17, 2026

@copilot please revert the changes to the files that were previously just symbolic links

Done in commit c1a3104. All 46 files that were previously symlinks (jgcafs_atmos_post_f000.ecf through jgcafs_atmos_post_f120.ecf and jgcdas_atmos_post_anl.ecf/jgcdas_atmos_post_f*.ecf) have been restored to their original symlink state. The changes (export model=gcafs and export HOMEglobal=${HOMEgcafs}) are only applied to the 19 non-symlink files that should be directly modified.

@CoryMartin-NOAA
Copy link
Contributor

@copilot can you revert the export model=gcafs changes back to just be model=gcafs?

Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 17, 2026

@copilot can you revert the export model=gcafs changes back to just be model=gcafs?

Done in commit 28f002c. All 18 modified .ecf scripts now have model=gcafs (without export).


model=gcafs
%include <head.h>
export HOMEglobal=${HOMEgcafs}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. I just have one question: when should ${} be used instead of %...%?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I also wasn't sure if this was the appropriate place in the script for this inclusion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a desire to move this around, I may suggest moving this down a few lines where NET and RUN get exported. I feel like it might be better there to have exports all grouped together instead of include,export,include,export unless there is something in the env that has a HOMEglobal call. Thinking long-term gw having it below NET could also allow for a future of HOMEglobal=HOME%NET% type call if needed, but for this specific implementation I don't know if this minor change is necessary.

I'm not an expert on the % % vs ${ } usage either. @CarlosMDiaz-NCO would probably have the best knowledge on the what the syntax for variables in the ecf scripts should be

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

% % variables are defined at the ecflow level and are usually used to keep the ecf scripts general. For example in hourly models we will set CYC at the hourly family level so all the jobs under it can export cyc=%CYC% and use the same job names and ecf scripts. Sometimes they can be used to set different resources in case different cycles of the same job have different usage.

If it's available you should use${ }because adding too many variables to the ecflow definition is more complicated to follow. But if there's some need to add NET and RUN at that level we can do it. This re-use of the ecf scripts this way only works if the jobs are really similar. Otherwise, if they need different modules or different variables it should be it's own ecf script.

@CoryMartin-NOAA CoryMartin-NOAA changed the title Inherit HOMEgcafs in ecf scripts and set HOMEglobal [GCAFSv1] Inherit HOMEgcafs in ecf scripts and set HOMEglobal Mar 19, 2026
@CoryMartin-NOAA
Copy link
Contributor

@copilot instead of adding this line around line 13, can you add it around line 28?

Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 20, 2026

@copilot instead of adding this line around line 13, can you add it around line 28?

Done in commit e3ffc32. export HOMEglobal=${HOMEgcafs} has been moved to immediately after export EXPDIR=%EXPDIR% in all 18 scripts, grouping it with the other exports.

Copy link
Contributor

@CoryMartin-NOAA CoryMartin-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we all agree that this location makes more sense? Thanks @TravisElless-NOAA for the suggestion

Copy link
Contributor

@TravisElless-NOAA TravisElless-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks @CoryMartin-NOAA

Copy link
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@CoryMartin-NOAA CoryMartin-NOAA merged commit 14912c3 into dev/gcafs.v1 Mar 20, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants