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

The values of soil parameters changes after refactoring Startinit #252

Open
Crystal-szj opened this issue Aug 22, 2024 · 7 comments
Open

Comments

@Crystal-szj
Copy link
Contributor

Hi @SarahAlidoost @yijian
I found that the refactoring StartInit causes some changes in soil properties parameters. I used the Plumber2 data AR-SLu site to do a test run, and prove that there are some changes due to refactoring as I mentioned here.

Although these changes occasionally didn't affect the results of AR-SLu due to the different soil depth settings (variables initND1, initND2, ... initND6), it did change the result in my case.

I put my case here for your information. In my case, the Tot_depth = 100, the value of initNDs are initND1 = 5, initND2 = 10, initND3 = 20, initND4 = 40, initND5 = 60, initND6 = 80. The number of soil layers is 29.

There are two changes I found for now:

  1. The value and length of IS changed: the length of IS changes from 29 (before refactor) to 30 (after refactor)
    image
    In this figure, the left part is the results of the old version (before refactoring), and the right part is the results after reforcing.

  2. The IS is the soil type index which is used to define the soil parameters for soil layers. The changes of IS cause different results for soil hydraulics parameters such as Ks, Theta_s, Theta_r. (The value of Theta_f should be changed also, since it didn't save in the old version, so I didn't show it in the following figure.)
    image
    In this figure, the first column is the no. of the soil layer, and the 2-4 columns in the left (before refactoring) and right (after refactoring) parts are the value of Ks, theta_s and theta_r, respectively.

The difference of soil hydraulic parameters changes the simulated soil moisture, and energy and carbon fluxes. In order to make the results of the two versions consistent, the soil hydraulic parameters in the two versions must be the same.
Please let me know if you need more information.

@SarahAlidoost
Copy link
Member

@Crystal-szj the issue with different sizes of IS is the value of the index in SoilVariables.IS(i), here and here. the index i should be j. Then, the size of SoilVariables.IS is 54. Can you please replace i with j and test if other results have also been fixed?

@Crystal-szj
Copy link
Contributor Author

Crystal-szj commented Aug 22, 2024

@Crystal-szj the issue with different sizes of IS is the value of the index in SoilVariables.IS(i), here and here. the index i should be j. Then, the size of SoilVariables.IS is 54. Can you please replace i with j and test if other results have also been fixed?

@SarahAlidoost Thanks for your reply. Do you mean I replace

J = SoilVariables.IS(i);

to

J = SoilVariables.IS(j)?

@SarahAlidoost
Copy link
Member

SarahAlidoost commented Aug 22, 2024

@Crystal-szj the issue with different sizes of IS is the value of the index in SoilVariables.IS(i), here and here. the index i should be j. Then, the size of SoilVariables.IS is 54. Can you please replace i with j and test if other results have also been fixed?

@SarahAlidoost Thanks for your reply. Do you mean I replace

J = SoilVariables.IS(i);

to

J = SoilVariables.IS(j)?

yes and also this one:
SoilVariables.IS(i) = indexOfSoilType;

to
SoilVariables.IS(j) = indexOfSoilType;

@Crystal-szj
Copy link
Contributor Author

@Crystal-szj the issue with different sizes of IS is the value of the index in SoilVariables.IS(i), here and here. the index i should be j. Then, the size of SoilVariables.IS is 54. Can you please replace i with j and test if other results have also been fixed?

@SarahAlidoost Thanks for your reply. Do you mean I replace

J = SoilVariables.IS(i);

to

J = SoilVariables.IS(j)?

yes and also this one: SoilVariables.IS(i) = indexOfSoilType;

to SoilVariables.IS(j) = indexOfSoilType;

Thanks @SarahAlidoost, It works. The soil parameters are the same now. Although the energy and carbon fluxes are not the same yet, I'll update it later if I found the reasons.

@SarahAlidoost
Copy link
Member

SarahAlidoost commented Aug 23, 2024

@Crystal-szj the issue with different sizes of IS is the value of the index in SoilVariables.IS(i), here and here. the index i should be j. Then, the size of SoilVariables.IS is 54. Can you please replace i with j and test if other results have also been fixed?

@SarahAlidoost Thanks for your reply. Do you mean I replace

J = SoilVariables.IS(i);

to

J = SoilVariables.IS(j)?

yes and also this one: SoilVariables.IS(i) = indexOfSoilType;
to SoilVariables.IS(j) = indexOfSoilType;

Thanks @SarahAlidoost, It works. The soil parameters are the same now. Although the energy and carbon fluxes are not the same yet, I'll update it later if I found the reasons.

@Crystal-szj great, can you please create a pull request with those two index-related changes and re-generate exe file? and when you find more fixes, you can create another pull request. Small pull requests are easier to review.

@SarahAlidoost SarahAlidoost added the bug Something isn't working label Aug 26, 2024
@SarahAlidoost
Copy link
Member

@Crystal-szj Have you seen the last comment above? can you please fix the indices and submit a pull request? so we can address some parts of this issue.

@Crystal-szj
Copy link
Contributor Author

Hi, @SarahAlidoost, Thank you for your suggestion. I have created a pull request accordingly #260.

SarahAlidoost added a commit that referenced this issue Sep 25, 2024
@SarahAlidoost SarahAlidoost removed the bug Something isn't working label Sep 26, 2024
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

No branches or pull requests

2 participants