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

Extend farquhar mechanism to spatially varying #774

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

imreddyTeja
Copy link
Contributor

@imreddyTeja imreddyTeja commented Sep 17, 2024

Purpose

Extend FarquharParemeters and OptimalityFarquharParameters to support a mechanism parameter that is
<: Union{FT, ClimaCore.Fields.Field}
Closes #762

To-do

  • Add to NEWS.md
  • Add to ClimaArtifacts repo

Content

This change is applied to both OptimalityFarquharParameters
and FarquharParameters. Previously, the mechanism parameter
was held as an empty struct of type ::C3 or ::C4 to allow
for dispatch. Because it is not possible to create a
ClimaCore field of different structs, the mechanism param is changed to a
is_c3 param that holds 1s and 0s. This choice was made because
reading a NetCDF file requires regridding, which must be performed over Numbers.
If the input data file must store the mechanism as a float, then it is easiest to
also internally represent the mechanism as a float. The C3/C4 structs
are now removed. The functions that previously used C3/C4 structs for dispatching
are converted to two seperate functions, which are called based on the is_c3 value.

If you have a better name for the parameter than is_c3, please suggest it. I previously considered
c3_mechanism_flag.

The constructor checks that the mechanism value of field is
between 0 and 1, and throws an error otherwise. Regridding a field of
0s and 1s may result in values between 0 and 1, so the constuctor rounds them
to the nearest integer. This implementation allows for future support of
mixed mechanisms in an area represented by proportion c3 without many changes.

All calls to the FarquharParameters constructor are changed to
work with the changes to mechanism representation. Tests are also
changed to work with new constructor and representation of mechanism.

The tests in canopy_model are changed to run twice using a for loop. The first run has all the parameters
as floats, and the second run has all canopy parameters that can be fields as fields. In this
case, the photosynthesis mechanism is tested as a field that alternates between 1.0 and 0.0,
which represents alternating between C3 and C4. The tests were previously all run over a Point
domain. The tests that use FarquharParameters are changed to run on a spherical surface.
Additionally, a test for vcmax as a field is added.

Land.jl and land_region.jl are changed to use the mechanism map from
the clm data.


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

Copy link
Member

@Sbozzolo Sbozzolo left a comment

Choose a reason for hiding this comment

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

Thank you!

Artifacts.toml Outdated Show resolved Hide resolved
experiments/benchmarks/land.jl Outdated Show resolved Hide resolved
experiments/long_runs/land_region.jl Outdated Show resolved Hide resolved
src/standalone/Vegetation/photosynthesis.jl Show resolved Hide resolved
@imreddyTeja imreddyTeja force-pushed the tr/extend-farquhar-mechanism-to-spatially-varying branch 2 times, most recently from b48308c to 9c7ab32 Compare September 17, 2024 00:49
ext/CreateParametersExt.jl Outdated Show resolved Hide resolved
"Photosynthesis mechanism: C3 only"
mechanism::C3
is_c3::MECH
Copy link
Collaborator

Choose a reason for hiding this comment

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

since we only allow for C3 right now with the Optimality model, should we @Assert all(is_c3 .== 1) in the constructor?

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 followed the behavior of the previous constructor, which ignores any kwarg for is_c3 and sets is_c3 to FT(1). Should this be changed?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see that now, I think that is fine. I guess someone could set up the parameters using the default constructor with is_c3 = 0, but we note that this only supports C3 for now in the doc string.

Copy link
Collaborator

@kmdeck kmdeck left a comment

Choose a reason for hiding this comment

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

very nice, thank you! I left some questions in the comments but otherwise this looks good to me.

This change is applied to both OptimalityFarquharParameters
and FarquharParameters. Previously, the mechanism parameter
was held as an empty struct of type ::C3 or ::C4 to allow
for dispatch. Because it is not possible to create a
ClimaCore field of different structs, the mechanism param is changed to a
is_c3 param that holds 1s and 0s. This choice was made because
reading a NetCDF file requires regridding, which must be performed over Numbers.
If the input data file must store the mechanism as a float, then it is easiest to
also internally represent the mechanism as a float. The C3/C4 structs
are now removed. The functions that previously used C3/C4 structs for dispatching
are converted to two seperate functions, which are called based on the is_c3 value.

The constructor checks that the mechanism value of field is
between 0 and 1, and throws an error otherwise. Regridding a field of
0s and 1s may result in values between 0 and 1, so the constuctor rounds them
to the nearest integer. This implementation allows for future support of
mixed mechanisms in an area represented by proportion c3 without many changes.

All calls to the FarquharParameters constructor are changed to
work with the changes to mechanism representation. Tests are also
changed to work with new constructor and representation of mechanism.

The tests in canopy_model are changed to run twice using a for loop. The first run has all the parameters
as floats, and the second run has all canopy parameters that can be fields as fields. In this
case, the photosynthesis mechanism is tested as a field that alternates between 1.0 and 0.0,
which represents alternating between C3 and C4. The tests were previously all run over a Point
domain. The tests that use FarquharParameters are changed to run on a spherical surface.
Additionally, a test for vcmax as a field is added.

Land.jl and land_region.jl are changed to use the mechanism map from
the clm data.
@imreddyTeja imreddyTeja force-pushed the tr/extend-farquhar-mechanism-to-spatially-varying branch from 9c7ab32 to 361b333 Compare September 17, 2024 18:32
@imreddyTeja imreddyTeja marked this pull request as ready for review September 17, 2024 18:47
@imreddyTeja imreddyTeja merged commit cedde2c into main Sep 17, 2024
12 checks passed
@imreddyTeja imreddyTeja deleted the tr/extend-farquhar-mechanism-to-spatially-varying branch September 17, 2024 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend Photosynthesis mechanism to support SpaceVaryingInput
3 participants