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

Model is not finalizing #111

Open
MostafaGomaa93 opened this issue Nov 13, 2024 · 8 comments · May be fixed by #116
Open

Model is not finalizing #111

MostafaGomaa93 opened this issue Nov 13, 2024 · 8 comments · May be fixed by #116

Comments

@MostafaGomaa93
Copy link
Contributor

Hello @BSchilperoort
I noticed that the model is not finalized with an error message if the model is running for longer periods (e.g. 20 days). I did several tests with different sites but always same problem. I actually can't figure out where is the problem, so would appreciate your help. I attached the model here in case you could test from your side.

NL-Loo_2024-11-13-1159.zip

Screenshot 2024-11-13 170345

@BSchilperoort
Copy link
Contributor

Maybe the model is encountering an error during finalization. Maybe something is going wrong trying to write data to file.

I am not sure if errors are written to a log file, this would be a good improvement for stemmus_scope as it'll make it easier to debug these issues.
e.g. https://nl.mathworks.com/help/matlab/ref/diary.html

@MostafaGomaa93
Copy link
Contributor Author

I run the model through Matlab (without Pystemmusscope) and it runs to the end successfully

@SarahAlidoost
Copy link
Member

I run the model through Matlab (without Pystemmusscope) and it runs to the end successfully

Just to clarify, did you run the model via matlab and bmi interface? the finalize is a bmi function and this block is where the function is used.

Another point, please check if your pystemmusscope installation is based on the main branch in this repo since the newest changes have not been released yet.

@MostafaGomaa93
Copy link
Contributor Author

MostafaGomaa93 commented Nov 14, 2024

Just to clarify, did you run the model via matlab and bmi interface? the finalize is a bmi function and this block is where the function is used.

If I run without BMI, the model runs successfully. If I run through BMI (using Bart notebook or mine), the final step model.finalize() gave an error (error message showed above). Through BMI, the model actually runs to the end, but not all the csv files are generated and the binary files are not deleted.

This link has a model files example (in case you could give it a try from your side)
NL-Loo_2024-11-07-1226.zip

@SarahAlidoost
Copy link
Member

SarahAlidoost commented Dec 9, 2024

@MostafaGomaa93 The finalize() function returns the ValueError: Model terminated with return code None because saving CSV files in MATLAB takes longer when dealing with large time series, and this process is slower than the Python process defined here. Here are a few ways to fix the issue:

  • Remove io.bin_to_csv() from the finalize block, defined here: This step is slow when handling longer time series. Instead, the binary-to-CSV conversion should happen after the model finishes, ideally in the PyStemmusScope.save module. MATLAB should only save CSV files for debugging purposes.
  • Delete unnecessary output files: see this issue.
  • Increase the sleep function time: It’s currently set to 10 seconds, but this setting can exposed to StemmusScopeBmi so users can adjust it. Note that increasing the sleep time and (hardcoding it) will slow down the model runtime overall. update: the setting can be added to config file instead of StemmusScopeBmi.

@SarahAlidoost
Copy link
Member

SarahAlidoost commented Dec 9, 2024

@MostafaGomaa93 as discussed, beside removing unnecessary output files, the setting can be added to config file. Here suggestion on how to use an optional variable SleepDuration from the config file:

        # set the sleep duration if it exists in the config file, otherwise default to
        # 10 seconds
        self.sleep_duration = int(config.get("SleepDuration", 10))
  • and later, it is used in this line as sleep(self.sleep_duration)

@MostafaGomaa93
Copy link
Contributor Author

MostafaGomaa93 commented Dec 9, 2024

  • Delete unnecessary output files: see this issue.

Many thanks, @SarahAlidoost. I commented the unnecessary large files in this PR

@MostafaGomaa93
Copy link
Contributor Author

I add those two lines also in this PR

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 a pull request may close this issue.

3 participants