Skip to content

Added new modules for Infiltration, Evapotranspiration, and soil moisture storage and movement.#147

Draft
djmallum wants to merge 65 commits intoChrismarsh:developfrom
djmallum:develop
Draft

Added new modules for Infiltration, Evapotranspiration, and soil moisture storage and movement.#147
djmallum wants to merge 65 commits intoChrismarsh:developfrom
djmallum:develop

Conversation

@djmallum
Copy link
Contributor

@djmallum djmallum commented Aug 26, 2024

This PR used to just be for infiltration but now is for all three new modules:

Evapotranspiration: Evapotranspiration_All.cpp

Infiltration: Infil_All.cpp

soil moisture storage and movement: soil_module.cpp

More details to come...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change can probably be rejected and is specific to the build on my mac

@djmallum djmallum changed the title Adding new infiltration module Added new modules for Infiltration, Evapotranspiration, and soil moisture storage and movement. Jul 16, 2025
@djmallum
Copy link
Contributor Author

@Chrismarsh

There are some minor issues with this commit. In particular see the change to src/core.cpp. I have zero intention of modifying core.cpp in this PR, however, in an attempt to merge this repositories develop into my own version prior to pushing this PR draft, some changes refused to properly update.

https://github.com/Chrismarsh/CHM/pull/147/files#diff-17c117fc6e5ca9905cfcc4961ff9ec4d3a3f733d8fa527d394ec9e2864dd03dc

From this blob, you can see my PR is trying to change the loop variables in these loops from size_t to global_ordinal_type. No matter how many times I retry the merge (Einstein insanity something something...), specifically resolving this conflict correctly so that size_t is the loop variable, my local repo refuses to actually keep them after the merge.

If you'd be willing to try applying this merge yourself, here is a back up of my develop branch prior to the attempted merge: https://github.com/Chrismarsh/CHM/pull/147/files#diff-17c117fc6e5ca9905cfcc4961ff9ec4d3a3f733d8fa527d394ec9e2864dd03dc

All I did was call

git fetch upstream/develop
git merge upstream/develop

And then resolved the various merges to match the upstream branch.

djmallum added 24 commits July 17, 2025 15:23
The following are based on the CRHM SoilX and K_Estimate modules. The
abstractions are specific to this project.

1. soil_base: a base soil class. Could add things here that are shared
   among all soil classes.
2. soil_two_layer: The main code body for dealing with mositure in the
   soil in its various layers.
3. soil_ET: Computes ET based on incoming actual ET computed elsewhere.
4. I_K_estimate: Interface for the K_estimate class (see below)
5. K_estimate: soil_two_layer expects a I_K_estimate object to compute
   the drainage factors between the various components of the soil.
   These factors are computed in K_estimate and through the interface of
   I_K_estimate.
6. soil_DTO: An object for the soil class here which contains all the
   variables and parameters used by each of the components above.
   Therefore, each of the above classes are stateless and rely on this
   component.
For computing runoff, subsurface runoff and dealing with infiltration.

Coordinates the submodules:

1. soil_two_layer
2. soil_ET
3. K_estimate
4. soil_DTO
Input from an ET module changed from potential_ET to actual_ET

Output of soil_ET changed from actual_ET to actual_soil_ET
Infil_All: Computes infiltration for winter-time (frozen) soils
following the CRHM module crack/Prairie_Infiltration/GreenCrack

Computes infiltration for unfrozen soils with two options:

1. Green-Ampt
2. Ayers

Green-Ampt follows the CRHM module GreenAmpt and Ayers follows the
unfrozen approach in Praire_Infiltration.

The soil data table is a class in the Soils namespace called soils_na.
Look at the body of the class for future documentation (does not
currently exist in a completed form).
ET base classes will be derived from evapbase. evapbase includes structs
for holding input variables/parameters (varbase) and one for a output variables (model_output).

varbase is expected to be derived by the submodules which derived from
evapbase. model_output can be derived if desired.
Had trouble updating CMake to 3.30, so minimum is changed in this commit
to 3.27 to get it to compile
`bool has_soil();`

This function checks if there is soil at the current triangle face. It
does so by checking the parameter `soil_storage_max`.

`
template <typename T>
T soil_attribute(const std::string variable);
`

Retrieves a soil attribute from the current triangle face.

`
template <typename T>
T soil_attribute(const std::string variable,const std::string category);
`

Identical to the previous. Used to connect to a look-up table. Some soil
attributes are written in the .json as
`
"soils": {
    "soil_type": {
        "0": "sand",
        "1": "loam",
        "2": "clay",
        "3": "loamsand",
        "4": "sandloam",
        "5": "siltloam",
        "6": "saclloam",
        "7": "clayloam",
        "8": "siclloam",
        "9": "sandclay",
        "10": "siltclay",
        "99": "pavement"
    },
    "soil_texture": {
        "0": "coarse_over_coarse",
        "1": "medium_over_medium",
        "2": "medium_over_fine",
        "3": "fine_over_fine",
        "4": "soil_over_bedrock"
    },
    "soil_groundcover": {
        "0" : "bare_soil",
        "1" : "row_crop",
        "2" : "poor_pasture",
        "3" : "small_grains",
        "4" : "good_pasture",
        "5" : "forested"
    }

}
`

Because of how CHM works, MESHER inputs are required to be integers.
These tables convert them to strings to be interpreted by
/src/physics/Soils.h soils_na class.
Input was potential_ET but this is not accurate, it is actual ET. So it
is changed to actual_ET.

Output from soil_ET is now changed from actual_ET to actual_soil_ET.

this change has already been included in a previous commit for soil_DTO: a432856f
Evapotranspiration_All: The ET module that computes the actual ET based
on either the PenmanMonteith class and the PriestleyTaylor class.

This version uses PriestlyTaylor if the surface is a lake and
PenmanMonteith otherwise. Could be more granular in the future.
Frozen soil calculations is now in infil_submodules/Crack.*

Unfrozen soil calculation using Ayers is now in infil_submodules/Ayers.*

Both need some level of refactoring
Added a check_map() function to help with debugging ayers_texture.

Added a lazy initialzer function so that there is only one copy per
process
Ayers and Crack have tests now.

tests/main.cpp has also been changed so that it can run. Since these
tests were last ran, there were significant changes to CHM and so the
old bits would not run.
Ayers and Crack infiltration submodules and tests for both and
associated directories added to CMAKE
djmallum added 26 commits July 17, 2025 15:23
This allows googletest to build gmock. I'm not sure if this is the
optimal branch but it works.
`get_dt()` no longer takes an input and instead uses `this->` to access
the local_module global_param object.

future version could simply just have the global_param object as a
pointer member on soil_DTO.
Really only useful for debugging tests
New submodules will make use of templates in order to allow compile time
flexibility.

The goal here is that a derived class of base_step will take a class as
its template parameter that is unique to each face.

The basic idea is as follows

```cpp
class step_API
{
public:
    double& get_input1();
    double& get_input2();
    double& get_input3();

    void set_output1(const double& out);
    void set_output2(const double& out);

    void reset_cache();
    void set_outputs();
private:
    double input1;
    double input2;
    double input3;

    double output1;
    double output2;
};

template<class T>
class calculation : base_step<T>
{
public:
    void execute() override final; // might skip vtable lookup
    explicit calculation(T& _d) : base_step<T>(_d);
    ~calculation();
private:
// store state variables here
};

template<class T>
void calculation::execute()
{
    double input1 = T.get_input1();

    set_output1(input1 * 2.0);
};
```

inputs/outputs private members are caches and get_input1 and get_input2
should be written to fetch from mesh_elem object on the first access and
from the cache on subsequent access. The reason we do this is because we
hope to develop a tool which sets order of operations at compile time
without having to fully write it out for every version.

output functions should use += as the calculation class does not know if
it is the first or last step.

Need to include functions which reset the cache between steps to 0.0.
Likewise, output functions should not set to the mesh_elem object
because there is no way to know at compile time which is called last.
Instead, create a set_outputs function or similar.

Avoiding virtual functions here to avoid vtable overhead in the step_API
class.

A final note:

Every module has a `data` class that persists between calls to
`run(mesh_elem& face)` and is unique to each triangle. It should be
written like this:

```cpp
class data
{
public:
    std::unique_ptr<calculation<stepAPI>> calc;

private:
    step_API api;
};
```

and during `init(mesh* domain)`

```cpp
//get data as d
calc = std::make_unique<calculation<stepAPI>>(d.api);
```

We cannot have the template parameter be `data` itself because then it
referes to an incomplete type and fails to compile. So we use
composition instead.

We use `calc` as a unique pointer so that we can delay initialization.
This has a cost of 8 bytes for the pointer address. This will add up to
8 MB on a million triangles (ignoring the overhead of the object
itself). Considered std::optional but this tends to have a higher
overhead per instance.
Due to template argument deduction, if the default value (second
argument) does not match the type of the variable (e.g., 2 vs 2.0) there
are precision misses if the value from file has a nonzero decimal value.

For example, if the default of wind_height is 2 but the value from file
is 8.6 the 0.6 will be lost and only 8.0 will be retained.
is_lake has been replaced by a variable member of soil_ET_DTO rather
than a function to avoid the circular pointer dependencies.

get_dt and get_new_day both no longer take an input and are adjusted in
soil_module for this purpose.
Creates an accumulator class. It must be bound via a pointer to another
variable with `bind_to_var(double& var)`. `execute()` is run every time
step and it automatically accumulates the bound variable which can be
modified elsewhere. Call `get_last_mean()` to extract the mean value
from the previous day when needed.

Expects the `template<class data>` to have two functions defined:
`steps_per_day()` and `is_new_day()`.

`is_new_day()`

checks if this is the start of a new day and initiates the computation
of the mean value.

`steps_per_day()`

```cpp
N = steps_per_day();
mean = accumulated_temperature / N;
```
soil_two_layer is refactored so calculations are encapsulated and more
easily testable.
Bug fixes to ET module and PenmanMonteith submodule
Adding new submodule XG algorithm and updating soil_module to use it
In the CRHM output data from marmot creek, upper clearing, the soil
module (SoilX) used a fixed value of porosity at 0.5.
@djmallum
Copy link
Contributor Author

@Chrismarsh

There are some minor issues with this commit. In particular see the change to src/core.cpp. I have zero intention of modifying core.cpp in this PR, however, in an attempt to merge this repositories develop into my own version prior to pushing this PR draft, some changes refused to properly update.

https://github.com/Chrismarsh/CHM/pull/147/files#diff-17c117fc6e5ca9905cfcc4961ff9ec4d3a3f733d8fa527d394ec9e2864dd03dc

From this blob, you can see my PR is trying to change the loop variables in these loops from size_t to global_ordinal_type. No matter how many times I retry the merge (Einstein insanity something something...), specifically resolving this conflict correctly so that size_t is the loop variable, my local repo refuses to actually keep them after the merge.

If you'd be willing to try applying this merge yourself, here is a back up of my develop branch prior to the attempted merge: https://github.com/Chrismarsh/CHM/pull/147/files#diff-17c117fc6e5ca9905cfcc4961ff9ec4d3a3f733d8fa527d394ec9e2864dd03dc

All I did was call

git fetch upstream/develop
git merge upstream/develop

And then resolved the various merges to match the upstream branch.

Resolved this issue by interactive rebase with --rebase-merges command and dropping some extra commits authored by @Chrismarsh that were deleted upstream by force push. See image below for commits that were dropped.

image

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 was getting an odd build warning with this one, even commented, so I removed the . But obviously didn't need to...

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.

1 participant