Redefine Evap and Trap arrays

[MostafaGomaa93]

Description

Please add a description of the changes proposed in the pull request.

closes #271
closes #274
relates #90
relates EcoExtreML/STEMMUS_SCOPE_Processing#115
relates EcoExtreML/STEMMUS_SCOPE_Processing#111

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.

[SarahAlidoost]

@MostafaGomaa93 please note when the changes are related to BMI, we need a new release of the model (in Semmus_scope repo). After release, a new docker image will be created, automatically. Then the version of the image should be fixed in EcoExtreML/STEMMUS_SCOPE_Processing#116

[SarahAlidoost]

@MostafaGomaa93 please also add changes to changelog file, it is one of the items in the pull request todo list, see here

[MostafaGomaa93]

Request changes is done

[Crystal-szj]

@bobzsu @yijianzeng relates #90, In this version, some of the large output files will no longer be saved anymore. Should we delete all those outputs?

@EcoExtreML/developers If anyone needs some of these outputs for a specific analysis, please comment here. Thank you very much for all your cooperation.

[SarahAlidoost]

@bobzsu @yijianzeng relates #90, In this version, some of the large output files will no longer be saved anymore. Should we delete all those outputs?

@ALL If anyone needs some of these outputs for a specific analysis, please comment here. Thank you very much for all your cooperation.

@ALL does not work in github. Instead, you can tag a team as @EcoExtreML/developers.

[yijianzeng]

@bobzsu @yijianzeng relates #90, In this version, some of the large output files will no longer be saved anymore. Should we delete all those outputs?

@ALL If anyone needs some of these outputs for a specific analysis, please comment here. Thank you very much for all your cooperation.

Hi @Crystal-szj & @EcoExtreML/developers, the principle is to keep all the original outputs of the current latest version, and if the new pull request is submitted for being merged, we just add new output to the original outputs.

[MostafaGomaa93]

Hi @Crystal-szj & @EcoExtreML/developers, the principle is to keep all the original outputs of the current latest version, and if the new pull request is submitted for being merged, we just add new output to the original outputs.

The large output files prevent the model from being finalised in BMI, so that's why we want to remove them. Especially, as Bob also mentioned in #90 , those large outputs are probably not used by anyone

[SarahAlidoost]

@bobzsu @yijianzeng relates #90, In this version, some of the large output files will no longer be saved anymore. Should we delete all those outputs?
@ALL If anyone needs some of these outputs for a specific analysis, please comment here. Thank you very much for all your cooperation.

Hi @Crystal-szj & @EcoExtreML/developers, the principle is to keep all the original outputs of the current latest version, and if the new pull request is submitted for being merged, we just add new output to the original outputs.

@yijianzeng and @Crystal-szj do you have suggestions on how to fix issues #90 and EcoExtreML/STEMMUS_SCOPE_Processing#111? Currently, all the outputs are stored in output.mat file. Therefore, we can skip storing some of the variables mentioned here as csv files, especially in the main loop of the model.

[yijianzeng]

Hi @Crystal-szj & @EcoExtreML/developers, the principle is to keep all the original outputs of the current latest version, and if the new pull request is submitted for being merged, we just add new output to the original outputs.

The large output files prevent the model from being finalised in BMI, so that's why we want to remove them. Especially, as Bob also mentioned in #90 , those large outputs are probably not used by anyone

@MostafaGomaa93 If this is only for BMI, for the time being, you can put a switch to comment out the output. However, for those who are not using BMI, this option should not be removed.

If you read carefully Bob's comments, there is a line
"Yunfei: please check if all ouputs are in output.mat, if so, you do not need all the csv files. needed ones can be extracted later."

[yijianzeng]

@bobzsu @yijianzeng relates #90, In this version, some of the large output files will no longer be saved anymore. Should we delete all those outputs?
@ALL If anyone needs some of these outputs for a specific analysis, please comment here. Thank you very much for all your cooperation.

Hi @Crystal-szj & @EcoExtreML/developers, the principle is to keep all the original outputs of the current latest version, and if the new pull request is submitted for being merged, we just add new output to the original outputs.

@yijianzeng and @Crystal-szj do you have suggestions on how to fix issues #90 and EcoExtreML/STEMMUS_SCOPE_Processing#111? Currently, all the outputs are stored in output.mat file. Therefore, we can skip storing some of the variables mentioned here as csv files, especially in the main loop of the model.

@SarahAlidoost Hi Sarah, if all are stored in 'output.mat' file, then it is fine. We can indeed skip saving those mentioned variables as csv files.

[Crystal-szj]

@yijianzeng and @Crystal-szj, do you have suggestions on how to fix issues #90 and EcoExtreML/STEMMUS_SCOPE_Processing#111? Currently, all the outputs are stored in output.mat file. Therefore, we can skip storing some of the variables mentioned here as CSV files, especially in the main loop of the model.

Hi @SarahAlidoost, thank you for your comments.
It seems to me that these variables that were saved in .csv files were not stored in outputs.mat. Only the values of the final step can be found in outputs.mat. However, that is not the case with "keep all the original outputs of the current latest version".

Could we set a specific storage option only when BMI is opened?

[SarahAlidoost]

@yijianzeng and @Crystal-szj, do you have suggestions on how to fix issues #90 and EcoExtreML/STEMMUS_SCOPE_Processing#111? Currently, all the outputs are stored in output.mat file. Therefore, we can skip storing some of the variables mentioned here as CSV files, especially in the main loop of the model.

Hi @SarahAlidoost, thank you for your comments. It seems to me that these variables that were saved in .csv files were not stored in outputs.mat. Only the values of the final step can be found in outputs.mat. However, that is not the case with "keep all the original outputs of the current latest version".

Could we set a specific storage option only when BMI is opened?

@Crystal-szj thanks for explaining. yes, as you suggested, it is possible to set an option for saving large files in csv format. @MostafaGomaa93 will help to implement this. Please note that this suggestion does not resolve issue #90, as the storage of large files (initially in binary format and later in CSV format) will remain unchanged.

[SarahAlidoost]

@MostafaGomaa93 thanks for the fixes. 👍

[MostafaGomaa93]

@SarahAlidoost
Thanks a lot for the comments and the suggestions

[MostafaGomaa93]

@Crystal-szj @yijianzeng

We (I and @SarahAlidoost) added a new option called FullCSVfiles in the config_file. The user can switch on/off this option to/not to save the large output csv files. So, using this option, the BMI issue will be solved.

[Crystal-szj]

@Crystal-szj @yijianzeng

We (I and @SarahAlidoost) added a new option called FullCSVfiles in the config_file. The user can switch on/off this option to/not to save the large output csv files. So, using this option, the BMI issue will be solved.

@MostafaGomaa93 @SarahAlidoost Great! Thanks for your efforts. Should I merge this PR to main branch?

[Crystal-szj]

@Crystal-szj @yijianzeng

We (I and @SarahAlidoost) added a new option called FullCSVfiles in the config_file. The user can switch on/off this option to/not to save the large output csv files. So, using this option, the BMI issue will be solved.

Hi @MostafaGomaa93 , I checked your changes, and I found that the new option FullCSVfiles is not included in the config_file_cirb.txt in add_Trap_to_BMI branch, and it is hardcoded in src/+io/read_config.m.
If I understand correctly, does it mean that the users have to change the src/+io/read_config.m to activate the new option?

[MostafaGomaa93]

Hi @MostafaGomaa93 , I checked your changes, and I found that the new option FullCSVfiles is not included in the config_file_cirb.txt in add_Trap_to_BMI branch, and it is hardcoded in src/+io/read_config.m. If I understand correctly, does it mean that the users have to change the src/+io/read_config.m to activate the new option?

The FullCSVfiles option is optional in the config_file (as implemented here). If the FullCSVfiles keyword is not in the config_file, then the default is to save all output files. If the FullCSVfiles keyword is in the config_file, and FullCSVfiles = 0, the large output csv files will not be saved

[SarahAlidoost]

Hi @MostafaGomaa93 , I checked your changes, and I found that the new option FullCSVfiles is not included in the config_file_cirb.txt in add_Trap_to_BMI branch, and it is hardcoded in src/+io/read_config.m. If I understand correctly, does it mean that the users have to change the src/+io/read_config.m to activate the new option?

The FullCSVfiles option is optional in the config_file (as implemented here). If the FullCSVfiles keyword is not in the config_file, then the default is to save all output files. If the FullCSVfiles keyword is in the config_file, and FullCSVfiles = 0, the large output csv files will not be saved

@Crystal-szj in addition to Mostafa's comment, a documentation has been added, see here

[Crystal-szj]

Hi @MostafaGomaa93 , I checked your changes, and I found that the new option FullCSVfiles is not included in the config_file_cirb.txt in add_Trap_to_BMI branch, and it is hardcoded in src/+io/read_config.m. If I understand correctly, does it mean that the users have to change the src/+io/read_config.m to activate the new option?

The FullCSVfiles option is optional in the config_file (as implemented here). If the FullCSVfiles keyword is not in the config_file, then the default is to save all output files. If the FullCSVfiles keyword is in the config_file, and FullCSVfiles = 0, the large output csv files will not be saved

@MostafaGomaa93 Thanks for your explanation.

Hi @MostafaGomaa93 , I checked your changes, and I found that the new option FullCSVfiles is not included in the config_file_cirb.txt in add_Trap_to_BMI branch, and it is hardcoded in src/+io/read_config.m. If I understand correctly, does it mean that the users have to change the src/+io/read_config.m to activate the new option?

The FullCSVfiles option is optional in the config_file (as implemented here). If the FullCSVfiles keyword is not in the config_file, then the default is to save all output files. If the FullCSVfiles keyword is in the config_file, and FullCSVfiles = 0, the large output csv files will not be saved

@Crystal-szj in addition to Mostafa's comment, a documentation has been added, see here

Hi @MostafaGomaa93 and @SarahAlidoost , thanks for the explanation. That makes sense. I have no further questions. Feel free to merge it into the main branch.