upload the original code of PG model

[DanyangYu]

Description

fix #275

Checklist

• Add a reference to related issues.
• @mentions of the person or team responsible for reviewing proposed changes.
• This pull request has a descriptive title.
• Code is written according to the guidelines.
• The checks by MISS_HIT style checker and
  linter, below the pull request, are
  successful (green).
• Documentation is available.
• Add changes to the changelog file under section Unreleased.
• Model runs successfully, see tests.
• Ask a meinatainer to re-generate exe file if matlab codes are changed. About how to create an exe file, see exe readme.

[DanyangYu]

All global variables have been removed. Please review the revisions.

[Crystal-szj]

Hi @DanyangYu, thank you very much for your efforts on the PG model. I left some suggestions for your reference. After you resolve all conversations, I'll create a new execute file for this branch.

[DanyangYu]

@Crystal-szj @MostafaGomaa93 Hi, Zengjing and Mostafa. I’ve added some code to bin_to_csv.m, but there are conflicts preventing the merge. I’ve tried several times to resolve the issue but haven’t been successful. Do you have any suggestions on how to fix this?

[Crystal-szj]

@Crystal-szj @MostafaGomaa93 Hi, Zengjing and Mostafa. I’ve added some code to bin_to_csv.m, but there are conflicts preventing the merge. I’ve tried several times to resolve the issue but haven’t been successful. Do you have any suggestions on how to fix this?

Hi @DanyangYu, I resolved the conflict. But the style check has not passed yet. Could you

• first git pull my changes to your own branch,
• then revise the code style according to the message provided by MISS_HIT checks
• git push your changes again?

You can also run the style checks locally, see here. However, I didn't do the style check locally, @MostafaGomaa93 might be able to share some experience.

[Crystal-szj]

Hi @DanyangYu, I noticed you added both cropgrowth.m and cropgrowth.asv files. Just a kind reminder that the file with the .asv suffix (for autosave) is a temporary file used to automatically back up the changes from the prior version you worked on. It is not necessary to add .asv file to the RP.

There are two options to address this:
Option 1:git add <specific file name>, then git commit -m <commit message> to only add the necessary file.
Option 2: add *.asv into the .gitignorefile to prevent these files from being tracked in the RP.

[SarahAlidoost]

Suggested change

fprintf(strcat('\n KT = ', num2str(KT), ' \r'));

[SarahAlidoost]

Please don't use print statements in the main loop or any other loop of the model. If users run the model for long time series, it could create a huge log output.

[DanyangYu]

Hi, Sarah. Thanks for your comments. I just want to test how many steps are simulated. I will delete it in the final version.

[MostafaGomaa93]

Hi @DanyangYu

I didn't review your changes yet, but I have a major question. Did you add WOFOST in such a way to be an option for the user to activate or deactivate?
I observed this (options.calc_vegetation_dynamic) in this line and it is read for the input_data.xlsx file (here)
So, if I understand correctly, the user needs to change the value of options.calc_vegetation_dynamic to 1 or 0 in the input_data.xlsx file to activate/deactivate WOFOST, right?

[DanyangYu]

Hi @DanyangYu

I didn't review your changes yet, but I have a major question. Did you add WOFOST in such a way to be an option for the user to activate or deactivate? I observed this (options.calc_vegetation_dynamic) in this line and it is read for the input_data.xlsx file (here) So, if I understand correctly, the user needs to change the value of options.calc_vegetation_dynamic to 1 or 0 in the input_data.xlsx file to activate/deactivate WOFOST, right?

Hi Mostafa, yes, you are right. I add an option in 'input_data.xlsx' file

[MostafaGomaa93]

Hi Mostafa, yes, you are right. I add an option in 'input_data.xlsx' file

Okay, in an earlier discussion with @SarahAlidoost. She suggested that any new configuration should be done in the config_file. And coupling new models to STEMMUS_SCOPE is better to be in the BMI mode. The advantage of BMI is that you can always make use of the updated codes of the coupled models. So, if WOFOST is updated (with new capabilities), then the BMI will allow the user to still use the updated version of WOFOST.

I am not sure if you have time to shift your code by activating/deactivating WOFOST to be in BMI mode and using the config_file instead of the input_data.xlsx or not.

If not, then it is okay, you can keep everything as it is, but we must update the documentation, so the user can know the activation/deactivation of WOFSOT is done through the input_data.xlsx

[MostafaGomaa93]

The most important checks we need to do (as suggested by @SarahAlidoost) are the following:

• the variables' name are clear,
• inputs and outputs of the functions are valid,
• no global variable,
• no unnecessary changes,
• the model still works if wofost is not activated
• no hardcoded values and indices in arrays,
• if something is not clear, there should be a comment explaining the decision made, in other words explaining "why" and not "what".
• the model can still work with octave and matlab version compatible with exe.

@DanyangYu, Can you please check your edits align with those checks above?

[Crystal-szj]

Hi Mostafa, yes, you are right. I add an option in 'input_data.xlsx' file

Okay, in an earlier discussion with @SarahAlidoost. She suggested that any new configuration should be done in the config_file. And coupling new models to STEMMUS_SCOPE is better to be in the BMI mode. The advantage of BMI is that you can always make use of the updated codes of the coupled models. So, if WOFOST is updated (with new capabilities), then the BMI will allow the user to still use the updated version of WOFOST.

I am not sure if you have time to shift your code by activating/deactivating WOFOST to be in BMI mode and using the config_file instead of the input_data.xlsx or not.

If not, then it is okay, you can keep everything as it is, but we must update the documentation, so the user can know the activation/deactivation of WOFSOT is done through the input_data.xlsx

Hi @MostafaGomaa93 In my opinion, Danyan didn't couple the whole WOFOST model into the STEMMUS-SCOPE, but only the crop/plant growth part. And all the codes he used were rewritten in Matlab (from Fortran).

@DanyangYu Feel free to correct me If I'm wrong.
Based on Mostafa's comments, I realized that we should rename the variables (such aswofost, wofostPar and functions) to distinguish between the 'plant growth module in STEMMUS-SCOPE' and 'WOFOST' model.
Let me know your opinion.