Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add jjob stub for prospective ocean analysis prep observation task #1831

Merged

Conversation

AndrewEichmann-NOAA
Copy link
Contributor

Description

Adds jjob and basic infrastructure for WCDA observation prep task, which will copy and process observations for the coupled ocean analysis. This is an initial addition, only a stub, to facilitate development of scripts that will do the work.

Partially resolves NOAA-EMC/GDASApp#579 and NOAA-EMC/GDASApp#590

Type of change

  • New feature (adds functionality)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? NO
  • Does this change require a documentation update? NO

How has this been tested?

Tested with GDASApp soca ctests in GDASApp branch feature/prepocniodaobs on Hera

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
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • I have made corresponding changes to the documentation if necessary

@AndrewEichmann-NOAA AndrewEichmann-NOAA marked this pull request as ready for review August 31, 2023 20:51
@AndrewEichmann-NOAA
Copy link
Contributor Author

FYI @guillaumevernieres

Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

mostly looks good.
See specific comments inline.
Missing is a jobs/rocoto/prepoceanobs.sh

@@ -0,0 +1,43 @@
#!/bin/bash
export STRICT="NO"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this NO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have an outstanding issue with variables not getting assigned (I suspect because we don't use them) - this is being worked on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,43 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets call this job:
JGLOBAL_PREP_OCEAN_OBS to be consistent with how jobs are named.
This job will be called for both gdas and gfs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

jobs/JGDAS_GLOBAL_OCEAN_ANALYSIS_PREP_OBS Outdated Show resolved Hide resolved
Comment on lines 11 to 12
# Ignore possible spelling error (nothing is misspelled)
# shellcheck disable=SC2153
Copy link
Contributor

Choose a reason for hiding this comment

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

What is being ignored?

Suggested change
# Ignore possible spelling error (nothing is misspelled)
# shellcheck disable=SC2153

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

jobs/JGDAS_GLOBAL_OCEAN_ANALYSIS_PREP_OBS Outdated Show resolved Hide resolved
jobs/JGDAS_GLOBAL_OCEAN_ANALYSIS_PREP_OBS Outdated Show resolved Hide resolved
@@ -0,0 +1,10 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer this config to be config.prepoceanobs
We currently have config.prep
Newer components would be config.prep<component>obs

Copy link
Contributor

Choose a reason for hiding this comment

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

update elsewhere as necessary.

@AndrewEichmann-NOAA
Copy link
Contributor Author

@aerorahul Thanks for the input - I'll get to it next week.

fi

##########################################
# Do not remove the Temporary working directory (do this in POST)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Do not remove the Temporary working directory (do this in POST)
# Remove the temporary working directory

Probably a residual from a previous copy-paste.
We do wan't to remove job specific run directories at the end of the job.

##########################################
# Do not remove the Temporary working directory (do this in POST)
##########################################
cd "${DATAROOT}" || exit 1
Copy link
Contributor

Choose a reason for hiding this comment

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

We do want to remove the data directory at the end of the job.

Suggested change
cd "${DATAROOT}" || exit 1
cd "${DATAROOT}" || (echo "FATAL ERROR: ${DATAROOT} does not exist. ABORT!"; exit 1)
[[ ${KEEPDATA} = "NO" ]] && rm -rf "${DATA}"

@AndrewEichmann-NOAA
Copy link
Contributor Author

New version @guillaumevernieres

Copy link
Contributor

@aerorahul aerorahul 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.

@aerorahul aerorahul merged commit 0a84646 into NOAA-EMC:develop Sep 11, 2023
4 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.

Prepare marine obs from the 6 hourly dump
2 participants