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

Bettina-ayse #221

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Bettina-ayse #221

wants to merge 26 commits into from

Conversation

jbathmann
Copy link
Collaborator

@jbathmann jbathmann commented Jan 31, 2023

This PR contains a first implementation of the soil water content modification for bettina called BETTINA-AYSE. For details see ODD protocol by heinermann/janosch.

Benchmark files are provided.

Tasks

@ChrisWudel
Copy link
Collaborator

Hi, I would like to do some simulations. How did you imagine the example simulations there is probably a lack of comparative data for growth, as I don't have any available at the moment.

@mcwimm
Copy link
Collaborator

mcwimm commented Feb 6, 2023

Hi, I would like to do some simulations. How did you imagine the example simulations there is probably a lack of comparative data for growth, as I don't have any available at the moment.

test data are provided in the benchmark folder (https://github.com/pymanga/pyMANGA/pull/221/files)

@ChrisWudel
Copy link
Collaborator

So, I have tested the "SoilWaterContent_CI.xml", "SoilWaterContent.xml" and "test_Benchmarks_CI.py" on Windows, and they work without any problems.
I will also carry out a test to see whether meaningful results are produced; I will contact Janosch about this.

Copy link
Collaborator

@mcwimm mcwimm left a comment

Choose a reason for hiding this comment

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

Code looks 👍 Although I did not understand everything at the first attempt. Can we chat about it later today?

@jbathmann
Copy link
Collaborator Author

@ChrisWudel Thank you for running the model!

@jbathmann
Copy link
Collaborator Author

Documentation added. YAPF used.

@mcwimm mcwimm mentioned this pull request Feb 18, 2023
6 tasks
Benchmarks/ExampleSetups/ExmouthGulf/python_script.py Outdated Show resolved Hide resolved
Benchmarks/ExampleSetups/ExmouthGulf/python_source.py Outdated Show resolved Hide resolved
Benchmarks/ExampleSetups/OGSExampleSetup/python_source.py Outdated Show resolved Hide resolved
Benchmarks/manual_Benchmarks.py Outdated Show resolved Hide resolved
## Measure for pore-size distribution
self._n = float(args.find("n").text)
## Suction pressure [hPa]
self._psi_matrix = float(args.find("psi_matrix").text)
Copy link
Collaborator

Choose a reason for hiding this comment

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

So far, we always changed everything to SI also if this differs from the ODD, see for example SimpleBettina.
I would prefer to do everything is SI units to be consistent.

## Maximal soil water content [m**3]
self._max_soil_water_content = (
self._omega_r + (self._omega_s - self._omega_r) / ((
1 + (-self._alpha * self._psi_matrix / 100)**self._n)**(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why "/100"?

exponent = 1 / (1 - 1 / self._n)
self._psi_matrix = - (
base ** exponent - 1) ** (1 / self._n) / self._alpha
self._psi_matrix[np.where(self._psi_matrix < -7860000)] = -7860000
Copy link
Collaborator

@mcwimm mcwimm May 4, 2023

Choose a reason for hiding this comment

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

Comment Janosch: Theoretical value representing very low water content (expon. pF-curve returns infinite big value)

@mcwimm mcwimm force-pushed the bettina_ayse branch 2 times, most recently from 70ae2a0 to eb0f706 Compare August 21, 2024 14:54
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.

3 participants