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

Review the constants bundling in pyrealm.constants #391

Open
davidorme opened this issue Jan 19, 2025 · 0 comments
Open

Review the constants bundling in pyrealm.constants #391

davidorme opened this issue Jan 19, 2025 · 0 comments

Comments

@davidorme
Copy link
Collaborator

davidorme commented Jan 19, 2025

Many of the constants in pyrealm.constants subclasses are presented as single float values, even when they are logical groups of coefficients from a single source. They would be more clearly documented and organised as tuples or dicts of values. For example:

# Mengoli 2023 soil moisture stress
soilm_mengoli_y_a: float = 0.62
"""Coefficient of the maximal level function for Mengoli soil moisture"""
soilm_mengoli_y_b: float = -0.45
"""Exponent of the maximal level function for Mengoli soil moisture"""
soilm_mengoli_psi_a: float = 0.34
"""Coefficient of the threshold function for Mengoli soil moisture"""
soilm_mengoli_psi_b: float = -0.60
"""Exponent of the threshold function for Mengoli soil moisture"""

I think this would be better as:

# Mengoli 2023 soil moisture stress
soilm_mengoli: dict[str, float] = field(
        default_factory=lambda: dict(y_a=0.62, y_b=-0.45, psi_a=0.34, psi_b=-0.60)
)
"""Coefficients of the Mengoil soil moisture penalty function for GPP.

The values are the coefficient ($y_a$) and exponent ($y_b$) of the maximal level function and 
the coefficient ($\psi_a$) and exponent $\psi_b$) of the threshold function.
"""

If we go down this route, it would be nice if we could use dot notation here - maybe moving to dotmap or python-box (aka Box) similar for these constant classes would be worthwhile? We could continue to use frozen dataclasses, but then we have to define subclasses for all the nested components, and this feels more like something where we want to define a Marshmallow validator and then expose in a more lightweight way?

Alternatively, this is simpler but you need to get the order right when using the values:

# Mengoli 2023 soil moisture stress
soilm_mengoli: tuple[float, ...] = (0.62, -0.45, 0.34, -0.60)
"""Coefficients of the Mengoil soil moisture penalty function for GPP.

The values are the coefficient ($y_a$) and exponent ($y_b$) of the maximal level function and 
the coefficient ($\psi_a$) and exponent $\psi_b$) of the threshold function and are given in that order.
"""
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

1 participant