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

Fix divide by zero for SWI #385

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

frostbytten
Copy link
Contributor

There is an equality check for SW(1) and SAT(1) on line 188, but there is no equality check on line 202. We have run into instances where SAT(1) == SW(1) and the model crashes. This patches it (and verifies the denominator is positive).

There is an equality check for SW(1) and SAT(1) on line 188, but there
is no equality check on line 202. We have run into instances where
SAT(1) == SW(1) and the model crashes. This patches it (and verifies the
denominator is positive).
@fabiooliveira72
Copy link
Contributor

Hi @frostbytten!

Could you please send an experimental files as example to reproduce this error?

I need to check if this will patch will not break into other conditions.

Thank you.

@fabiooliveira72
Copy link
Contributor

Hi @frostbytten

Looking at this without running an experiment I got to this conclusion:

This code is redundant because if SW is less than DUL then SW must be less than SAT. Because SAT is always greater than DUL.
You mentioned previously that it runs into issues where SAT == SW. However, this condition will not pass the IF statement above and SWI will be 1.0. Let is assume the IF statement passes, then you triggered a division by zero, because (SAT(1)-SW(1)) will be equal and set to zero for the division.

I assume your issue is somewhere else.

I hope you can have a experiment that we could run too and then we find a fix together!

Thank you.

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 this pull request may close these issues.

None yet

2 participants