WIP: Speed up singlediode._lambertw #1661

cwhanse · 2023-02-13T18:20:39Z

Closes Accelerate singlediode._lambertw #1649
I am familiar with the contributing guidelines
Tests added
~~[ ] Updates entries in docs/sphinx/source/reference for API changes.~~
Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for 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`).
New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
Pull request is nearly complete and ready for detailed review.
Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

Implements and documents the solution technique illustrate in this gist for solving for the maximum power point using the Lambert W method.

cwhanse · 2023-02-13T22:53:43Z

Coverage drops because I have not (yet) removed the now-unused private function which was maximized to find the MPP with the golden mean search. Assuming this PR is approved, that helper function can be removed before merging.

cwhanse · 2023-02-14T17:05:05Z

@pvlib/pvlib-maintainer I can't get the environment to solve to run the benchmark. Is it simple for one of you to run the new benchmark and suggest any edits/corrections?

mikofski · 2023-02-14T17:29:05Z

I can't get the environment to solve to run the benchmark

If you're referring to the conda environment, have you tried Mambaforge? Mamba is an improved solver that replaces conda. It usually solves environments faster and with less clashes in difficult environments. I have been using it for a little over a year, and IME it is a significant improvement over conda. If you prefer, you can just install mamba in your base miniconda/anaconda environment by using conda install -c conda-forge mamba rather switching to Mambaforge. Or if you have the latest conda, there is an experimental option to use libmamba since conda-4.12, but it is an older version, I found the experience poor, and inconsistent compared with the version of mamba from conda-forge.

kandersolar · 2023-02-14T17:35:42Z

Just looking at the CI, the quick-benchmarks jobs is erroring with ImportError: cannot import name 'Generator' from 'numpy.random' with numpy=1.16.5. It looks like Generator was added in numpy 1.17.0, so I guess we'll need to either bump the numpy version in the ASV configuration, or not use Generator in the benchmarks. Either way is fine with me.

cwhanse · 2023-02-14T17:49:29Z

I can rewind to numpy 1.16, it shouldn't affect the time for the benchmarks.

cwhanse · 2023-02-14T20:01:55Z

Just looking at the CI, the quick-benchmarks jobs is erroring with ImportError: cannot import name 'Generator' from 'numpy.random' with numpy=1.16.5. It looks like Generator was added in numpy 1.17.0, so I guess we'll need to either bump the numpy version in the ASV configuration, or not use Generator in the benchmarks. Either way is fine with me.

I can't find where that error was happening - the CI report for the asv action looks green to me. Did you run that locally (whcih I haven't been able to do), or where should I be looking?

It doesn't appear that this new benchmark is being run yet.

kandersolar · 2023-02-14T20:26:01Z

I almost never run the benchmarks locally. I vaguely recall having problems getting them working on Windows. I'm pretty sure I've run them locally on Linux before. The nightly benchmark server runs some flavor of Linux.

Here's the ASV run for 063abf8 which has the error I mentioned: https://github.com/pvlib/pvlib-python/actions/runs/4168577294/jobs/7215540457

I guess the job is (mis)configured in such a way that python errors in the benchmarks don't propagate out to become a CI job failure, so we always get the green checkmark here on the PR. That needs to be fixed.

It doesn't appear that this new benchmark is being run yet.

I think asv identifies benchmarks by looking for functions/methods whose name begins with time_, so renaming the benchmarks from bishop88 -> time_bishop88 should get it working.

kandersolar

Haven't inspected the math yet

docs/sphinx/source/user_guide/singlediode.rst

benchmarks/benchmarks/singlediode.py

Co-authored-by: Kevin Anderson <[email protected]>

kandersolar

I have now worked through (most of) the math myself.

Shall we delete _golden_sect_DataFrame and _pwr_optfcn if they aren't used anywhere anymore?

Also can we change the URLs for reference 3 in user_guide/singlediode.rst? One of them seems to be behind a Sandia login page and the other is Research Gate. Better alternatives:

kandersolar · 2023-02-18T16:29:45Z

pvlib/singlediode.py

+                  args=args)
+    v_mp = _lambertw_v_from_i(resistance_shunt, resistance_series, nNsVth,
+                              i_mp, saturation_current, photocurrent)
+    p_mp = i_mp * v_mp

    # Find Imp using Lambert W
    i_mp = _lambertw_i_from_v(resistance_shunt, resistance_series, nNsVth,


I think this i_mp calculation is now redundant and can be nixed.

kandersolar · 2023-02-18T17:48:15Z

pvlib/singlediode.py

+    if np.any(idx_z):
+        # explicit solution for gsh=0
+        t = il[idx_z] + io[idx_z] - i[idx_z]
+        res[idx_z] = i[idx_z] / t - np.log(t / io[idx_z])


When I derive this by hand I get an extra 2*i*rs/a term. Am I making a mistake? With the goal of finding a root of $\frac{dP}{dI}=V + I \frac{dV}{dI}$, start with the single diode equation:

$$ I = I_L - I_0 \left(\exp \left(\frac{V + I R_s}{n Ns V_{th}} \right) - 1 \right) - \frac{V + I R_s}{R_{sh}} $$

In the limit $R_{sh}\rightarrow\infty$, the last term vanishes:

$$ I = I_L - I_0 \left(\exp \left(\frac{V + I R_s}{n Ns V_{th}} \right) - 1 \right) $$

This can be rearranged to solve explicitly for $V$:

$$ V = n Ns V_{th} \log \left(\frac{I_L + I_0 - I}{I_0}\right) - I R_s $$

Differentiating $V$ with respect to $I$ yields

$$ \frac{dV}{dI} = - \frac{n Ns V_{th}}{I_L + I_0 - I} - R_s $$

Hence:

$$ \frac{dP}{dI} = n Ns V_{th} \log \left(\frac{I_L + I_0 - I}{I_0}\right) - I \frac{n Ns V_{th}}{I_L + I_0 - I} - 2 I R_s $$

The derivative is zero at $I_{mp}$, thus:

$$ \log \left(\frac{I_L + I_0 - I_{mp}}{I_0}\right) - \frac{I_{mp}}{I_L + I_0 - I_{mp}} - 2\frac{I_{mp} R_s}{n Ns V_{th}} = 0 $$

You're correct, I dropped a sign change.

benchmarks/benchmarks/singlediode.py

kandersolar · 2023-02-18T19:00:00Z

pvlib/singlediode.py

+                       (gsh * a)))
+    lambertwterm = np.array(lambertw(argW).real)
+
+    idx_inf = np.logical_not(np.isfinite(lambertwterm))


This will cause both nan and inf to be recalculated, which I'm guessing isn't desirable if the goal is to only recalculate for inf

kandersolar · 2023-02-18T19:11:09Z

pvlib/singlediode.py

+    if np.any(idx_z):
+        # explicit solution for gsh=0
+        t = il[idx_z] + io[idx_z] - i[idx_z]
+        res[idx_z] = 2. / t + i[idx_z] / t**2.


Depending on whether my previous comment regarding a missing term is in error, this may also need an additional term

cwhanse · 2023-02-20T19:35:11Z

Shall we delete _golden_sect_DataFrame and _pwr_optfcn if they aren't used anywhere anymore?

I vote 'yes'

cwhanse · 2023-02-22T23:26:58Z

Moving this back to WIP, because I'm finding convergence problems with corner cases: Rsh=infinity, IL=0. Back to paper and pen.

cwhanse added 8 commits February 13, 2023 11:17

switch lambertw mpp to newton technique

c294e40

handle Rsh=inf

62a8996

document edits

a97b383

pacify stickler

d84c83c

doc fixes

e6d0a33

whatsnew

e1aff50

add a benchmark

d362d7b

stickler, doc edits

063abf8

cwhanse changed the title ~~WIP: speed up singlediode._lambertw~~ Speed up singlediode._lambertw Feb 14, 2023

revert to numpy 1.16

5a0c80b

cwhanse added 4 commits February 14, 2023 14:08

rename benchmark functions

9e88b91

more doc edits

91ecf1c

fix stuff

6098885

fix errors

0ca692c

kandersolar reviewed Feb 15, 2023

View reviewed changes

docs/sphinx/source/user_guide/singlediode.rst Outdated Show resolved Hide resolved

docs/sphinx/source/user_guide/singlediode.rst Outdated Show resolved Hide resolved

benchmarks/benchmarks/singlediode.py Show resolved Hide resolved

Apply suggestions from code review

4da7d2f

Co-authored-by: Kevin Anderson <[email protected]>

kandersolar reviewed Feb 18, 2023

View reviewed changes

cwhanse changed the title ~~Speed up singlediode._lambertw~~ WIP: Speed up singlediode._lambertw Feb 24, 2023

Lakshyadevelops mentioned this pull request Mar 23, 2023

version of scipy upgraded to 1.5.0 in asv #1704

Closed

4 tasks

cwhanse mentioned this pull request Oct 10, 2023

Add "logwright" SDE solver functions #1884

Draft

8 tasks

WIP: Speed up singlediode._lambertw #1661

Are you sure you want to change the base?

WIP: Speed up singlediode._lambertw #1661

Uh oh!

Conversation

cwhanse commented Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cwhanse commented Feb 13, 2023

Uh oh!

cwhanse commented Feb 14, 2023

Uh oh!

mikofski commented Feb 14, 2023

Uh oh!

kandersolar commented Feb 14, 2023

Uh oh!

cwhanse commented Feb 14, 2023

Uh oh!

cwhanse commented Feb 14, 2023

Uh oh!

kandersolar commented Feb 14, 2023

Uh oh!

kandersolar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kandersolar left a comment

Choose a reason for hiding this comment

Uh oh!

kandersolar Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

kandersolar Feb 18, 2023

Choose a reason for hiding this comment

Uh oh!

cwhanse Feb 20, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kandersolar Feb 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kandersolar Feb 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cwhanse commented Feb 20, 2023

Uh oh!

cwhanse commented Feb 22, 2023

Uh oh!

Uh oh!

cwhanse commented Feb 13, 2023 •

edited

Loading

kandersolar Feb 18, 2023 •

edited

Loading

kandersolar Feb 18, 2023 •

edited

Loading