-
Couldn't load subscription status.
- Fork 1.1k
Use scipy's find_minimum in lambertw for MPP #2567
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
Conversation
|
What do you think of this approach? |
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 see quite a nice speedup locally (maybe 80% reduction in computation time for pvsystem.singlediode), nice!
pvlib/singlediode.py
Outdated
| if res.success.all(): | ||
| v_mp = res.x | ||
| p_mp = -1.*res.f_x | ||
| else: | ||
| use_gs = True |
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 am inclined to not include this safety net; if we plan to get rid of the GS function eventually, might as well let any unforeseen issues with the scipy approach bubble to the surface now so that we can address them sooner rather than later.
Or is there reason to expect the scipy function to fail sometimes?
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 can't think of a reason for failure other than bad inputs. Because res.success is a vector, we could pass over to the GS only those index values which failed, for whatever reason. Or skip the success check altogether and just push v_mp forward. I worry a little that will forward unconverged, but still numeric, values.
|
|
||
| # Find the voltage, v_mp, where the power is maximized. | ||
| # Start the golden section search at v_oc * 1.14 | ||
| p_mp, v_mp = _golden_sect_DataFrame(params, 0., v_oc * 1.14, _pwr_optfcn) |
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.
Just curious... I wonder if anyone knows why 1.14 is the magic number. I see it goes back to the early early days of pvlib-python.
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.
That was 1.14 * Voc_ref, the STC value of Voc. The factor of 1.14 was meant to push the right endpoint out when temperature is lower than STC.
In pvlib, v_oc reflects temperature and irradiance. I included 1.01 * v_oc in case the algorithm needs to start with a negative value of power at the right endpoint, since it is minimizing negative power. Now that I write this, I don't think that's necessary - it is provable that power has a unique maximum between v=0 and v=v_oc.
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 was wondering why it is 1.14 * Voc_ref rather than (for example) 1.15 * Voc_ref or 1.13 * Voc_ref or 1.2 * Voc_ref. Maybe computed from some suitable low temperature and a representative temperature coefficient? Just idle curiosity.
pvlib/singlediode.py
Outdated
| _pwr_optfcn) | ||
|
|
||
| # Find Imp using Lambert W | ||
| i_mp = _lambertw_i_from_v(v_mp, **params) |
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.
It's not really related to this PR, but can't we avoid an unnecessary calculation here and just do i_mp = p_mp / v_mp?
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.
Also unrelated: since we removed the ivcurve_pnts kwarg from singlediode (several versions ago), should we also delete that capability from _lambertw?
| params['resistance_shunt'], | ||
| params['nNsVth'],)) | ||
| v_mp = res.x | ||
| p_mp = -1.*res.f_x |
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.
Test failure is because the test expects i_mp=0 for photocurrent=0, but currently it's getting back i_mp=nan on some platforms. Maybe because photocurrent=0 violates the requirement of init:
The abscissae of a standard scalar minimization bracket. A bracket is
valid if arraysx1, x2, x3 = initsatisfyx1 < x2 < x3and
func(x1) >= func(x2) <= func(x3), where one of the inequalities
is strict.
We could detect photocurrent==0 and skip the optimizer for those cases?
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
…into golden_opt
|
@cwhanse FYI it looks like this branch got mixed up with the CEC/PVsyst parameter conversion code. |
|
Ugh. Starting over. |
optimize.elementwisefunctions #2497docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.Switches to the old golden section if
find_minimumis not available.Since
find_minimumreplaces the golden mean search, if scipy>=1.15, the current tests seem sufficient.