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.

Sign up for GitHub

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

Jump to bottom

Add fire blending to GOCART #2883

Open
wants to merge 69 commits into
base: develop
Choose a base branch
from
Open

Add fire blending to GOCART #2883

wants to merge 69 commits into from

Conversation

bbakernoaa
Copy link
Contributor

Description

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 a breaking change (a change in existing functionality)? YES/NO
  • Does this change require a documentation update? YES/NO
  • Does this change require an update to any of the following submodules? YES/NO (If YES, please add a link to any PRs that are pending.)
    • EMC verif-global
    • GDAS
    • GFS-utils
    • GSI
    • GSI-monitor
    • GSI-utils
    • UFS-utils
    • UFS-weather-model
    • wxflow

How has this been tested?

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

@bbakernoaa bbakernoaa marked this pull request as ready for review September 13, 2024 13:05
@bbakernoaa bbakernoaa marked this pull request as draft September 13, 2024 14:03
@WalterKolczynski-NOAA WalterKolczynski-NOAA changed the title [GOCART] Add Fire Blending Add fire blending to GOCART Sep 17, 2024
@aerorahul aerorahul added CI-Gaea-Ready **CM use only** PR is ready for CI testing on Gaea and removed CI-Gaea-Ready **CM use only** PR is ready for CI testing on Gaea labels Sep 19, 2024
@AntonMFernando-NOAA @bbakernoaa
Cleanup job for GEFS (NOAA-EMC#2919)
b22d7bc 
As forecast ensemble jobs are added to the global workflow, this PR
ensures the output is being cleaned up properly once it is no longer
needed.
  Resolves NOAA-EMC#833
@BoCui-NOAA @bbakernoaa
Update config.resources for bufr sounding job postsnd (NOAA-EMC#2917)
9965857 
This PR updates the parm/config/gfs/config.resources and env/WCOSS2.env
files for the BUFR sounding job "postsnd." It includes adjustments to
resource settings such as tasks per node and memory allocations for
various GFS resolutions, including C768, C1152, and others.

Here are the proposed changes:

C768: 7 nodes, 21 tasks per node
C1152: 16 nodes, 9 tasks per node
@mingshichen-noaa @bbakernoaa
Update global atmos upp job to use COMIN/COMOUT (NOAA-EMC#2867)
5b57604 
NCO has requested that each COM variable specify whether it is an input
or an output. This completes that process for the global-workflow
Unified Post Processor (UPP) task.
Refs: NOAA-EMC#2451
@KateFriedman-NOAA @bbakernoaa
Update to obsproc/v1.2.0 and prepobs/v1.1.0 (NOAA-EMC#2903)
1f95b2b 
This PR updates the `develop` branch to use the newer operational
`obsproc/v1.2.0` and `prepobs/v1.1.0`. The obsproc/prepobs installs in
glopara space on supported platforms use tags cut from the `dev/gfsv17`
branches in the respective repos. The installation of `prepobs/v1.1.0`
on WCOSS2 is called "gfsv17_v1.1.0" to help avoid GFSv16 users using it
instead of the operational module.

Also, the `HOMEobsproc` path is updated to set an empty default for
`obsproc_run_ver`. This both removes the need to set a default (and
constantly update it, which is duplication) and avoid the unset variable
error when the fcst jobs use their own load module script that does not
know `obsproc_run_ver`:
```
export HOMEobsproc="${BASE_GIT:-}/obsproc/v${obsproc_run_ver:-}"
```

This PR also reverts the prepobs and fit2obs installs on MSU back to the
glopara space from the temporary `/work/noaa/global/kfriedma/glopara`
space installs.

Lastly, this PR also includes updates to complete issue NOAA-EMC#2844 (merge
`build.spack.ver` and `run.spack.ver`).

Resolves NOAA-EMC#2291
Resolves NOAA-EMC#2840
Resolves NOAA-EMC#2844
@bbakernoaa
Merge remote-tracking branch 'remotes/upstream/develop' into feature/…
eb8bc78 
…fire_blending
@aerorahul
Merge branch 'develop' into feature/fire_blending
9a9a874
aerorahul
aerorahul requested changes Sep 25, 2024
View reviewed changes
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.

@bbakernoaa
Please let me know if any of my comments need clarifications

parm/config/gefs/config.prep_emissions Show resolved Hide resolved
parm/prep/aero_emissions.yaml Outdated Show resolved Hide resolved
parm/prep/aero_emissions.yaml Outdated Show resolved Hide resolved
parm/prep/aero_emissions.yaml Outdated Show resolved Hide resolved
parm/prep/aero_emissions.yaml Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Show resolved Hide resolved
@aerorahul
Copy link
Contributor

@bbakernoaa
Others, please don't hold this PR for my review. I think I have reviewed this PR and will let others review and provide comments.

WalterKolczynski-NOAA
WalterKolczynski-NOAA requested changes Oct 23, 2024
View reviewed changes
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA left a comment

Choose a reason for hiding this comment

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

Let me know if you have questions about the refactoring to keep the data inside the class (or anything else).

parm/config/gefs/config.prep_emissions Show resolved Hide resolved
ush/forecast_postdet.sh
# link directory containing GOCART input dataset, if provided
if [[ -n "${AERO_INPUTS_DIR}" ]]; then
${NLN} "${AERO_INPUTS_DIR}" "${DATA}/ExtData"
status=$?
[[ ${status} -ne 0 ]] && exit "${status}"
fi

# Link blending emissions if AERO_EMIS_FIRE == blending
if [[ "${AERO_EMIS_FIRE}" == "blending" && "${RUN}" == "gefs" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for the check against $RUN? Presumably, if $AERO_EMIS_FIRE is blending, we want to use blending regardless of the $RUN.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure... Rahul asked to add this but

Copy link
Contributor

Choose a reason for hiding this comment

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

Since this was only GEFS related, I suggested the RUN condition check. Pretty sure, SFS will need that as well in the future.
I can go either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather rely on the app config to know whether it wants blending.

ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
scripts/exglobal_prep_emissions.py Outdated Show resolved Hide resolved
scripts/exglobal_prep_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
aerorahul
aerorahul reviewed Nov 4, 2024
View reviewed changes
ush/forecast_postdet.sh
# link directory containing GOCART input dataset, if provided
if [[ -n "${AERO_INPUTS_DIR}" ]]; then
${NLN} "${AERO_INPUTS_DIR}" "${DATA}/ExtData"
status=$?
[[ ${status} -ne 0 ]] && exit "${status}"
fi

# Link blending emissions if AERO_EMIS_FIRE == blending
if [[ "${AERO_EMIS_FIRE}" == "blending" && "${RUN}" == "gefs" ]]; then
${NCP} "${COMOUT_CHEM_HISTORY}/${RUN}.${vdate:0:8}.${RUN}.blended_emissions.nc" "${DATA}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is vdate coming from in this function scope?

WalterKolczynski-NOAA
WalterKolczynski-NOAA requested changes Nov 7, 2024
View reviewed changes
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA left a comment

Choose a reason for hiding this comment

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

Mostly minor stuff in the second pass, but there are a couple suggestions from my previous review that were marked as resolved without being addressed (I marked them back as unresolved).

ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/forecast_postdet.sh
# link directory containing GOCART input dataset, if provided
if [[ -n "${AERO_INPUTS_DIR}" ]]; then
${NLN} "${AERO_INPUTS_DIR}" "${DATA}/ExtData"
status=$?
[[ ${status} -ne 0 ]] && exit "${status}"
fi

# Link blending emissions if AERO_EMIS_FIRE == blending
if [[ "${AERO_EMIS_FIRE}" == "blending" && "${RUN}" == "gefs" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather rely on the app config to know whether it wants blending.

ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated
--------
None
"""
Config_dict = self.task_config['config']
Copy link
Contributor

Choose a reason for hiding this comment

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

Object instances should be lowercase.

Suggested change
Config_dict = self.task_config['config']
config_dict = self.task_config['config']

Copy link
Contributor

Choose a reason for hiding this comment

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

This line got changed, but the subsequent references to it weren't.

ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
ush/python/pygfs/task/aero_emissions.py Outdated Show resolved Hide resolved
bbakernoaa and others added 5 commits November 12, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DavidHuber-NOAA DavidHuber-NOAA DavidHuber-NOAA left review comments

@WalterKolczynski-NOAA WalterKolczynski-NOAA WalterKolczynski-NOAA requested changes

@aerorahul aerorahul aerorahul requested changes

@CoryMartin-NOAA CoryMartin-NOAA CoryMartin-NOAA left review comments

@AntonMFernando-NOAA AntonMFernando-NOAA Awaiting requested review from AntonMFernando-NOAA

@GwenChen-NOAA GwenChen-NOAA Awaiting requested review from GwenChen-NOAA GwenChen-NOAA is a code owner

@DavidNew-NOAA DavidNew-NOAA Awaiting requested review from DavidNew-NOAA DavidNew-NOAA is a code owner

@RussTreadon-NOAA RussTreadon-NOAA Awaiting requested review from RussTreadon-NOAA RussTreadon-NOAA is a code owner

@KateFriedman-NOAA KateFriedman-NOAA Awaiting requested review from KateFriedman-NOAA KateFriedman-NOAA is a code owner

@TerrenceMcGuinness-NOAA TerrenceMcGuinness-NOAA Awaiting requested review from TerrenceMcGuinness-NOAA TerrenceMcGuinness-NOAA is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@bbakernoaa @aerorahul @CoryMartin-NOAA @WalterKolczynski-NOAA @DavidHuber-NOAA @AntonMFernando-NOAA @BoCui-NOAA @mingshichen-noaa @KateFriedman-NOAA