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

spatially varying and consistent biogeochemistry #690

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

kmdeck
Copy link
Member

@kmdeck kmdeck commented Jul 5, 2024

Purpose

The only parameters of the biogeochemsitry model which are spatially varying currently are those related to soil properties. This PR makes them spatially varying (i.e. supporting parameter fields), and it moves them to the "soil driver struct".

We then make the soil driver struct in the LandModel constructor using the soil parameters themselves, which enforces consistency

To-do

Content

  • Move porosity, theta_a100, and b out of the SoilCO2 Parameters
  • Move them into PrescribedMet, PrognosticMet (what we call the "soil driver" for Biogeochemistry)
  • access them via the driver in the soil CO2 update aux function, rather than from params struct
  • add check in integrated model constructor that they are the same - not needed, the constructor handles this.
  • update tests

  • I have read and checked the items on the review checklist.

@kmdeck kmdeck linked an issue Jul 5, 2024 that may be closed by this pull request
@kmdeck kmdeck self-assigned this Jul 5, 2024
@kmdeck kmdeck added this to the O2.3.5 Global run on GPU milestone Jul 5, 2024
@kmdeck kmdeck force-pushed the kd/spatially_varying_biogeochemistry branch from 2a953fe to d763267 Compare July 7, 2024 20:15
@kmdeck kmdeck marked this pull request as ready for review July 8, 2024 00:15
@kmdeck kmdeck requested a review from AlexisRenchon July 8, 2024 00:45
Copy link
Member

@AlexisRenchon AlexisRenchon left a comment

Choose a reason for hiding this comment

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

I have read through.
I think this is great! Thank you!
I don't have comments for the PR (ok to merge), but we will talk about it in person

@kmdeck kmdeck force-pushed the kd/spatially_varying_biogeochemistry branch from e1e316e to 6484ab7 Compare July 11, 2024 16:18
@kmdeck kmdeck added Run benchmarks Add this label to run benchmarks on clima enhancement New feature or request LSMv1 labels Jul 11, 2024
@kmdeck kmdeck force-pushed the kd/spatially_varying_biogeochemistry branch from ddadc2b to 42c41a9 Compare July 11, 2024 16:27
@kmdeck kmdeck enabled auto-merge July 11, 2024 16:39
@kmdeck kmdeck merged commit 5928728 into main Jul 11, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request LSMv1 Run benchmarks Add this label to run benchmarks on clima
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spatially varying biogeochemistry
2 participants