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

[WIP] Added support for importing mass flux from ExtData #172

Closed
wants to merge 10 commits into from

Conversation

LiamBindle
Copy link
Contributor

@LiamBindle LiamBindle commented Oct 18, 2021

This PR includes 5 major changes:

  1. Added support for driving GCHP with native GEOS metfields. #164
  2. Changes tracer advection to total VMR rather than dry VMR. The conversion to/from total VMR is done at the start/end of AdvCoreGridComp::Run().
  3. Added IMPORT_MASS_FLUX_FROM_EXTDATA as a field in GCHP.rc. If .true., GCHPenvGridComp imports MF[XY]C and C[XY]C from ExtData and converts them to real8's. If .false., GCHPenvGridComp calculates MF[XY]C and C[XY]C from wind and pressure. Both ways export MF[XY] and C[XY] real8's which are imported in AdvCoreGridComp.
  4. Refactored GCHPenvGridComp::Run to improve readability.
  5. [WIP] Add diagnostic for upwards mass flux #96

These are meant to be a clean and up-to-date implementation of the original mass flux simulation whose diff is here.

…ed to/from total VMR at start/end of AdvCoreGridComp::Run). Added IMPORT_MASS_FLUX_FROM_EXTDATA field to GCHP.rc. If true, MF[XY]C and C[XY]C are imported from ExtData and converted to real8 in GCHPenvGridComp. If false, MF[XY]C and C[XY]C are calculated online in GCHPenvGridComp from wind and pressure.
@LiamBindle
Copy link
Contributor Author

LiamBindle commented Oct 18, 2021

So far I've tested this with 24-hour transport tracer simulations driven by GEOS-IT data (a wind-based simulation versus a mass-flux-based simulation).

I still need to add a "mass flux simulation", as of bc42091 the necessary code-updates should be there.

@sdeastham Could you review this diff when you have a chance? A thorough check-over of the diffs to AdvCore_GridCompMod.F90 would be greatly appreciated. You can ignore the diffs to geos-chem and HEMCO submodules since those are from #164, and the diff to GCHP_GridCompMod.F90 is just plumbing.

For reference, here is the diff of the original mass flux simulation which is what I based this PR on.

@LiamBindle LiamBindle changed the title [WIP] Add support for importing mass flux from ExtData [WIP] Added support for importing mass flux from ExtData Oct 18, 2021
@stale
Copy link

stale bot commented Nov 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days it will be closed. You can add the "never stale" tag to prevent the Stale bot from closing this issue.

@stale stale bot added the stale No recent activity on this issue label Nov 18, 2021
@sdeastham
Copy link
Contributor

sdeastham commented Nov 18, 2021

I've reviewed this pull request and can confirm that it appears to work as intended.

EDIT: Never mind! Came across a potential issue while reviewing.

@stale stale bot removed the stale No recent activity on this issue label Nov 18, 2021
Copy link
Contributor

@sdeastham sdeastham left a comment

Choose a reason for hiding this comment

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

I think that the computation of mass fluxes when winds are read in is incorrect, since all advection is now performed using total VMRs rather than dry VMRs.

@lizziel
Copy link
Contributor

lizziel commented Nov 18, 2021

I have a general question about the description of this PR. Tracer advection uses mass mixing ratio so why is the term VMR used?

@lizziel
Copy link
Contributor

lizziel commented Nov 18, 2021

Is there an FVdycore PR in the works for the changes to AdvCore_GridComp?

@LiamBindle
Copy link
Contributor Author

Thanks Seb and Lizzie for your reviews! I'll make those updates early next week.

Regarding VMR vs mass mixing ratio, that's just a terminology mistake on my part. I just meant to differentiate between total and dry mixing ratios.

Regarding a PR for AdvCore_GridComp, I'm not planning to.

@lizziel
Copy link
Contributor

lizziel commented Nov 18, 2021

Why not do a PR for FVdycore? When resolving conflicts down the road during upstream version update it might useful to have around.

LiamBindle and others added 8 commits November 23, 2021 13:35
… (not dry) and this commit catches one that was missing.
…PORT and _EXPORT suffixes. Removed DryPLE imports and exports (advection uses total pressure now). Replumbed GCHPctmEnv-AdvCore-GEOSChem imports/exports.
… indentation, removed stylizing comments, removed unnecessary alignment whitespace, removed old docgen strings; more inline with modern MAPL style).
…utines that prepare (1) PLE exports, (2) the SPHU export, and (3) the mass flux exports.
@LiamBindle
Copy link
Contributor Author

@lizziel I did a pretty extensive refactor of GCHPctmEnv_GridComp (summarized in #181 if you want to see the diff). I merge it into this PR. I added the _IMPORT and _EXPORT suffixes, removed quite a bit of legacy code that wasn't used anymore, and I also reformatted the file. Could you take a look when you have a chance and let me know what you think?

@lizziel
Copy link
Contributor

lizziel commented Dec 14, 2021

Looks good. See my review at #181.

@stale
Copy link

stale bot commented Jan 14, 2022

This issue has been automatically marked as stale because it has not had recent activity. If there are no updates within 7 days it will be closed. You can add the "never stale" tag to prevent the Stale bot from closing this issue.

@stale stale bot added the stale No recent activity on this issue label Jan 14, 2022
@LiamBindle LiamBindle removed the stale No recent activity on this issue label Jan 17, 2022
@LiamBindle LiamBindle changed the base branch from docs/dev to dev January 17, 2022 19:41
@LiamBindle
Copy link
Contributor Author

Closed in favor of #190

@LiamBindle LiamBindle closed this Jan 17, 2022
@LiamBindle LiamBindle deleted the feature/massflux-simulation branch March 21, 2022 17:10
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.

None yet

3 participants