Skip to content

[develop] Fix unbound variable issue.#612

Merged
MichaelLueken merged 3 commits into
ufs-community:developfrom
danielabdi-noaa:bugfix/unbound
Feb 14, 2023
Merged

[develop] Fix unbound variable issue.#612
MichaelLueken merged 3 commits into
ufs-community:developfrom
danielabdi-noaa:bugfix/unbound

Conversation

@danielabdi-noaa
Copy link
Copy Markdown
Collaborator

@danielabdi-noaa danielabdi-noaa commented Feb 11, 2023

DESCRIPTION OF CHANGES:

Currently if you grep for unbound variables in your experiement directories log files, you will find many BASH_SOURCE: unbound variables messages, which may cause confusion when one is debugging a problem. The culprit files are filesys_cmds_vrfy.sh, print_msg.sh, change_cases.sh and preamble.sh. This PR fixes these files, so there should not be any unbound variables in the log files now.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • 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
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss2.intel
  • NOAA Cloud (indicate which platform)
  • Jenkins
  • fundamental test suite
  • comprehensive tests (specify which if a subset was used)

DEPENDENCIES:

None

DOCUMENTATION:

Not needed

ISSUE:

CHECKLIST

  • My code follows the style guidelines in the Contributor's Guide
  • I have performed a self-review of my own code using the Code Reviewer's Guide
  • I have commented my code, particularly in hard-to-understand areas
  • My changes need updates to the documentation. I have made corresponding changes to the documentation
  • My changes do not require updates to the documentation (explain).
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

LABELS (optional):

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

  • Work In Progress
  • bug
  • enhancement
  • documentation
  • release
  • high priority
  • run_ci
  • run_we2e_fundamental_tests
  • run_we2e_comprehensive_tests
  • Needs Cheyenne test
  • Needs Jet test
  • Needs Hera test
  • Needs Orion test
  • help wanted

CONTRIBUTORS (optional):

Copy link
Copy Markdown
Collaborator

@MichaelLueken MichaelLueken left a comment

Choose a reason for hiding this comment

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

@danielabdi-noaa I was able to run the fundamental tests on Jet using your fork. Using:

grep BASH_SOURCE *

on the expt_dirs logs, there are no unbound variable messages. Thanks for working to remove these messages from the logs! Approving now.

@MichaelLueken MichaelLueken added ci-hera-intel-WE Kicks off automated workflow test on hera with intel run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests labels Feb 13, 2023
@venitahagerty venitahagerty removed the ci-hera-intel-WE Kicks off automated workflow test on hera with intel label Feb 13, 2023
@venitahagerty
Copy link
Copy Markdown
Collaborator

venitahagerty commented Feb 13, 2023

Machine: hera
Compiler: intel
Job: WE
Repo location: /scratch1/BMC/zrtrr/rrfs_ci/autoci/pr/1237719822/20230213173512/ufs-srweather-app
Build was Successful
Rocoto jobs started
Long term tracking will be done on 10 experiments
If test failed, please make changes and add the following label back:
ci-hera-intel-WE
Experiment Succeeded on hera: pregen_grid_orog_sfc_climo
Experiment Succeeded on hera: community_ensemble_2mems_stoch
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v15p2
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_GSMGFS_lbcs_GSMGFS_suite_GFS_v15p2
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_RAP_suite_HRRR
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_2017_gfdlmp_regional_plot
Experiment Succeeded on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_HRRR
Experiment Succeeded on hera: grid_RRFS_CONUScompact_25km_ics_HRRR_lbcs_RAP_suite_RRFS_v1beta
Experiment Succeeded on hera: MET_ensemble_verification
Experiment Succeeded on hera: grid_RRFS_CONUS_25km_ics_FV3GFS_lbcs_FV3GFS_suite_GFS_v16
All experiments completed

Comment thread .cicd/Jenkinsfile
values 'cheyenne', 'gaea', 'hera', 'jet', 'orion' //, 'pclusternoaav2use1', 'azclusternoaav2eus1', 'gclusternoaav2usc1'
// Uncomment line below to re-add use of Hera
// values 'cheyenne', 'gaea', 'hera', 'jet', 'orion' //, 'pclusternoaav2use1', 'azclusternoaav2eus1', 'gclusternoaav2usc1'
values 'cheyenne', 'gaea', 'jet', 'orion' //, 'pclusternoaav2use1', 'azclusternoaav2eus1', 'gclusternoaav2usc1'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@MichaelLueken, just for clarification, why would we want to remove an option to run Jenkins tests on Hera?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@JeffBeck-NOAA Previously, EPIC was allowed to use the nems account on Hera by EMC. Unfortunately, the use of the account on Hera has been rescinded about a week ago. When this happened, the platform team shutdown the Jenkins runner on Hera to ensure that no PRs are run on the machine. The Jenkinsfile still attempts to submit jobs to Hera, which are forcefully aborted, leading to failure due to abort with the Jenkins tests. It is best to temporarily comment out Hera to keep the tests from failing due to abort. Once EPIC acquires a new account for Hera, it will be easy to reactivate the Jenkins pipeline again.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@JeffBeck-NOAA Simply put, EPIC is no longer able to run jobs on Hera. EPIC has turned off the ability for Jenkins to run on Hera. Since Hera is down in Jenkins, tasks that attempt to launch on Hera are forcefully aborted, leading to failures in the Jenkins tests (an example of this is @mkavulich's PR #600, the details for the continuous-integration/jenkins/pr-merge says The build of this commit was aborted).

This modification will ensure that jobs for Hera are no longer added to the build queue (as noted from the continuous-integration/jenkins/pr-merge entry passing in the PR). Once EPIC has an account that it can use to launch jobs again, then the Jenkins runner on Hera will be restarted, and I will be able to uncomment out the Hera entries in the .cicd/Jenkinsfile.

Please let me know if you have any questions.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, @MichaelLueken! It's extremely unfortunate that EPIC no longer has the ability to run Jenkins on Hera, since it's such a critical machine to support. Do you know why EPIC turned off access? Does Jenkins use a specific project that is no longer available?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@JeffBeck-NOAA I think the issue is that EPIC was utilizing EMC's nems account, but EMC decided that EPIC would no longer be able to use the nems account last week (it was quite surprising and all of EPIC is reeling from this decision). Since Jenkins is maintained by EPIC, it is required to use EPIC's account on the various Tier-1 machines. With the loss of the nems account, Jenkins is no longer allowed to run on Hera. Once a new account for EPIC is set up on Hera, the Jenkins runner on Hera will be reactivated, the account will be updated, and then I will be able to restore Hera in the .cicd/Jenkinsfile.

Copy link
Copy Markdown
Collaborator

@JeffBeck-NOAA JeffBeck-NOAA Feb 13, 2023

Choose a reason for hiding this comment

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

Thanks for the background, @MichaelLueken. I just sent an email to a couple people at EMC to get the history behind this change and to see what we can do. I cc'd you. It's critical that we can continue automated tests on Hera for both the RRFS and for our DTC-based regional testing and evaluation work which is all conducted on that platform.

@MichaelLueken
Copy link
Copy Markdown
Collaborator

If you have the time, please review this PR and @mkavulich PR #600. These two bug fixes are important to get into develop as soon as possible. Thank you very much for your time.

@MichaelLueken MichaelLueken merged commit f3ed8ef into ufs-community:develop Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run_we2e_coverage_tests Run the coverage set of SRW end-to-end tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unbound variables

5 participants