-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix ZeroDivisionError in simple_efficiency when load_loss=0 #2646
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
base: main
Are you sure you want to change the base?
Fix ZeroDivisionError in simple_efficiency when load_loss=0 #2646
Conversation
pvlib/transformer.py
Outdated
| if load_loss == 0: | ||
| return input_power - no_load_loss * transformer_rating | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if load_loss == 0: | |
| return input_power - no_load_loss * transformer_rating |
I don't think this is necessary with the alternate form of the quadratic equation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have removed it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a test with load_loss=0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!! @cwhanse
i have added a test covering the load_loss=0 that checks the linear-limit behavior P_out = P_in − L_no_load * P_nom
please let me know if you would prefer this test to also cover array like inputs or assert behavior at additional edge cases (negative input power)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep the scope to addressing the division by zero issue. Test looks good.
Please open a new issue to extend testing to np.array and pd.Series types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @cwhanse
sure i will open issues separately for each np.array and pd.Series
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @cwhanse sure i will open issues separately for each
np.arrayandpd.Series
Just one issue for both of them types is enough.
fixes a
ZeroDivisionErrorinpvlib.transformer.simple_efficiencywhen
load_loss = 0.when
load_lossis zero, the transformer loss model reduces to a linearrelationship rather than a quadratic one. this PR handles that case
explicitly to avoid division by zero while preserving existing behavior
for
load_loss > 0.Fixes #2645