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

SoilTemp2 quadruples the running time of some simulations #9467

Open
ilhuber opened this issue Nov 25, 2024 · 1 comment · May be fixed by #9468
Open

SoilTemp2 quadruples the running time of some simulations #9467

ilhuber opened this issue Nov 25, 2024 · 1 comment · May be fixed by #9468
Labels
Improvement An enhancement to an existing functionality or system Refactor

Comments

@ilhuber
Copy link
Contributor

ilhuber commented Nov 25, 2024

Where is the code located?

SoilTemperature (aka SoilTemp2)

What needs to be changed?

Three issues have a negative impact on the running time of SoilTemp2:

  1. doThomas creates a new string for each layer twice for each soiltemp2 timestep (48 timesteps per simulation day) when calling boundCheck, but This string is often unused. Inlining the bound checking and only creating the string when the check is failed should solve this.
  2. The value of latitude read from a traditional met file is obtained from Convert.ToDouble each time it is needed by the model, which is frequently in the case of SoilTemp2. Caching this value in some way ought to provide relief here.
  3. The depth above each layer is computed every timestep, but could similarly be cached.

Why does the code need changing?

Switching from CERESSoilTemperature to SoilTemperature more than quadruples the running time of a large (approx 9 million 34 year simulations with 18 layer soil profiles) factorial. Some amount of slowdown should be expected, but this amount seems excessive.

@ilhuber ilhuber added Improvement An enhancement to an existing functionality or system Refactor labels Nov 25, 2024
@ilhuber ilhuber changed the title SoilTemp2 SoilTemp2 quadruples the running time of some simulations Nov 25, 2024
@ilhuber ilhuber linked a pull request Nov 25, 2024 that will close this issue
@ilhuber
Copy link
Contributor Author

ilhuber commented Dec 2, 2024

In the context of my simulation the changes in the linked pull request are worth about a 40% speedup, though you would expect less for a simulation with a smaller soil profile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement An enhancement to an existing functionality or system Refactor
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

1 participant