-
Notifications
You must be signed in to change notification settings - Fork 1.1k
ENH: add methods and tests for a explicit IV curve calculation of single-diode model #409
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
Changes from 62 commits
22d2e5c
bb7c49a
36d4f78
817a303
b17a8cd
0666d29
595e254
5ade588
7179096
4b66f87
664d7d8
09a9e14
06be522
1aba56a
f09be91
4905363
a3d2bb4
8ee1b94
c9a893a
c5248bb
68c65c3
41e0c83
d7b6e62
aa3d29a
9fc350d
54e8d18
16ad9b4
7aca302
086e73f
9f2b157
555e946
52a8e88
c0e18a5
09c6a75
ff8cc0b
446fa9e
f976f61
db88022
a509dd3
62010c2
5c939b8
66a801d
df1423b
eb20cf4
e3805ef
5f89578
ba84fcf
0f3893c
d6023b1
80b3352
ef20676
98d1c01
4ecd913
bf05d2d
edc445d
f542d15
cc6c976
22c53fc
442a3d4
6bcffe8
e6b60c6
0fc9c83
fc6cee1
28c8ffb
58361c1
5f9ed41
43475cc
c79ab97
1ad6031
f14ba04
8d86560
337d7b4
ce5b0f5
3e009f8
082dfd5
ceb69cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
# Byte-compiled / optimized / DLL files | ||
.pytest_cache/ | ||
__pycache__/ | ||
*.py[cod] | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,7 +65,6 @@ algorithm. | |
|
||
spa | ||
|
||
|
||
Correlations and analytical expressions for low precision solar position | ||
calculations. | ||
|
||
|
@@ -80,6 +79,7 @@ calculations. | |
solarposition.equation_of_time_pvcdrom | ||
solarposition.hour_angle | ||
|
||
|
||
Clear sky | ||
========= | ||
|
||
|
@@ -205,6 +205,23 @@ Functions relevant for the single diode model. | |
pvsystem.i_from_v | ||
pvsystem.singlediode | ||
pvsystem.v_from_i | ||
pvsystem.mpp | ||
pvsystem.estimate_voc | ||
pvsystem.bishop88 | ||
|
||
Low-level functions to support :func:`pvlib.pvsystem.bishop88`. *Note*: | ||
:func:`pvlib.singlediode_methods.bishop88` and | ||
:func:`pvlib.singlediode_methods.estimate_voc` are also imported into the | ||
``pvlib.pvsystem`` module. | ||
|
||
.. autosummary:: | ||
:toctree: generated/ | ||
|
||
singlediode_methods.estimate_voc | ||
singlediode_methods.bishop88 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @cwhanse you once asked for a more specific function name. Does encapsulating the function within a singlediode module address that concern? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm OK with |
||
singlediode_methods.bishop88_i_from_v | ||
singlediode_methods.bishop88_v_from_i | ||
singlediode_methods.bishop88_mpp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "methods" makes me think of methods vs. functions but maybe that's just me. probably best to go with it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was my thought, also, but in this case I think it's OK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can change this to "how" I think that's what Pandas often uses. Is there any precedent for PVLIB? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. LOL! I meant the keyword, but I'm fine with everything as it is if you are. |
||
|
||
SAPM model | ||
---------- | ||
|
@@ -231,7 +248,6 @@ PVWatts model | |
pvsystem.pvwatts_ac | ||
pvsystem.pvwatts_losses | ||
|
||
|
||
Other | ||
----- | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,9 +11,37 @@ API Changes | |
|
||
Enhancements | ||
~~~~~~~~~~~~ | ||
* Add sea surface albedo in irradiance.py (:issue:`458`) | ||
* Implement first_solar_spectral_loss in modelchain.py (:issue:`359`) | ||
* Clarify arguments Egref and dEgdT for calcparams_desoto (:issue:`462`) | ||
* Add sea surface albedo in ``irradiance.py`` (:issue:`458`) | ||
* Implement :meth:`~pvlib.modelchain.ModelChain.first_solar_spectral_loss` | ||
in ``modelchain.py`` (:issue:`359`) | ||
* Clarify arguments ``Egref`` and ``dEgdT`` for | ||
:func:`~pvlib.pvsystem.calcparams_desoto` (:issue:`462`) | ||
* Implement a reliable "gold" implementation of the single diode model (SDM) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These additions to whatsnew seem out of date.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I exposed the two new main functions, I'll go with the flow, but perhaps that additional documentation and API change can be added in another PR, perhaps the one where we move the Lambert-W methods to that module? I'll just add some notes to what's new, that I added this module and put some low level plumbing in there, okay? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Let's batch move to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still find this whatsnew text to be confusing.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✔️ |
||
using a bisection method (Brent, 1973) bounded by points known to include the | ||
full forward-bias 1st quadrant IV-curve. Also implement a "fast" method using | ||
the Newton-Raphson method that is not bounded, but should be safe for well | ||
behaved IV-curves. (:issue:`408`) | ||
* Implement :func:`~pvlib.pvsystem.bishop88` for explicit calculation of | ||
arbitrary IV curve points using diode voltage instead of cell voltage. This | ||
method is called for ``ivcurve_pnts`` in :func:`~pvlib.pvsystem.singlediode` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of
If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✔️ |
||
if the method is either ``'newton'`` or ``'brentq'``, but the IV curve points | ||
will be log spaced instead of linear. | ||
* Implement :func:`~pvlib.pvsystem.estimate_voc` to estimate open circuit | ||
voltage by assuming :math:`R_{sh} \to \infty` and :math:`R_s=0` as an upper | ||
bound in bisection method for :func:`~pvlib.pvsystem.singlediode`. | ||
* Combine existing :func:`~pvlib.pvsystem.singlediode` method using Lambert-W | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bullet should be 2nd in the list:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved this first before explanation of each method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✔️ |
||
with the two new implementations to form a wrapper, that takes an additional | ||
``method`` argument ``('lambertw', 'newton', 'brentq')`` that defaults to a | ||
Lambert-W solution. Selecting either "brentq" or "newton" as the method uses | ||
``bishop88`` with the corresponding search method. (:issue:`410`) | ||
* Add :func:`~pvlib.pvsystem.mpp` method to compute the max power point using | ||
the new "brentq" bounded bisection method. | ||
* Add new module ``pvlib.singlediode_methods`` which contains low-level | ||
functions to support implementing ``bishop88`` and various other methods like | ||
``bishop88_i_from_v``, ``bishop88_v_from_i``, and ``bishop88_mpp``. *Note*: | ||
:func:`pvlib.singlediode_methods.bishop88` and | ||
:func:`pvlib.singlediode_methods.estimate_voc` are also imported into the | ||
``pvlib.pvsystem`` module. | ||
|
||
|
||
Bug fixes | ||
|
@@ -48,5 +76,6 @@ Contributors | |
* Will Holmgren | ||
* Yu Cao | ||
* Cliff Hansen | ||
* Mark Mikofski | ||
* Alan Mathew | ||
|
Large diffs are not rendered by default.
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 recommend against importing these functions into
pvlib.pvsystem
, especially if we're thinking of moving the lambertw function out of it. It's much easier to go one way than the other.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 for the decisiveness.