Skip to content

Add partial shading example using new reverse bias functionality #968

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

Merged
merged 11 commits into from
Jul 17, 2020

Conversation

kandersolar
Copy link
Member

@kandersolar kandersolar commented May 19, 2020

  • I am familiar with the contributing guidelines
  • 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`).
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels and Milestone are assigned to the Pull Request and linked Issue.

I was checking out the new reverse bias functionality to teach myself about cell-level modeling and thought it might make a good example. Here is the first pass. In addition to whether people think this is a good addition to the example gallery, I'd also be interested in any feedback about the validity of the approach in general just for my own learning.

Built version: https://pvlib-python--968.org.readthedocs.build/en/968/auto_examples/plot_partial_module_shading_simple.html

)
vd = np.linspace(0.99*kwargs['breakdown_voltage'], v_oc, ivcurve_pnts)

ivcurve_i, ivcurve_v, _ = singlediode.bishop88(vd, *sde_args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative is to use singlediode.bishop88_i_from_v, which accepts the cell voltage rather than the diode voltage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I admit I didn't think carefully about this line. What is the reasoning to use one over the other in this context? Strictly speaking, is the breakdown voltage a cell voltage or a diode voltage?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breakdown voltage is compared with diode (or junction) voltage. Cell voltage is after series resistance etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, should have been more clear -- I'm on board with breakdown voltage being related to the junction rather than the cell as a whole, but I was referring to the "breakdown voltage" value that would be included in a cell parameter dictionary. If it is inferred from measured IV curves and not adjusted for the series resistance voltage drop, it would be a cell voltage no? Not that it matters here, just curious for myself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is inferred from measured IV curves

I believe this is the case.

and not adjusted for the series resistance voltage drop, it would be a cell voltage no?

That probably depends on how the value was extracted from the data. If the value was extracted by fitting the single diode equation including the reverse bias term, then the parameter should be viewed as a junction voltage.

return f_interp(i)


def combine(dfs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some code to submit to pvlib.ivtools that will include a function add_series.

@cwhanse
Copy link
Member

cwhanse commented May 19, 2020

The calculation process looks correct to me. If you have the enthusiasm for it, a plot of one of the IV curves would be interesting. You might also consider accounting for a bypass diode on the modeled string.

@mikofski
Copy link
Member

Have you considered using scipy.constants for elementary charge (qe) and Boltzmann constant (kB)?

from scipy.constants import e as qe, k as kB

>>> qe
# 1.602176634e-19

>>>: kB
# 1.380649e-23

Alternately, you can take the temperature insensitive approximation and just keep the thermal voltage (Vth) at 26[mV]. Even though Vth depends on temperature, the diode equation itself is much less sensitive to Vth than it is the cubic temperature dependence of the dark current I0.

IMHO the temperature dependence of Vth is misleading to new PV students and leads them in the wrong direction because it's effect is opposite that of I0.


kb = 1.380649e-23 # J/K
qe = 1.602176634e-19 # C
Vth = kb * (273.15+25) / qe
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

units of V for thermal voltage to be consistent?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also maybe giving charge in units of Joules per volt [J/V] makes this conversion more intuitive to the reader?

@mikofski
Copy link
Member

mikofski commented May 19, 2020

What do you think is going on in this plot?
image
It looks like there are two levels of shading?

Ah, I get it now, that's the "partially" cells in between the full shade and full lit cells. Cool!

Not to beat a dead horse, but for a module, IMO it doesn't make sense to show the reverse bias past the bypass diode trigger voltage, around -0.5[V].

@kandersolar
Copy link
Member Author

What do you think is going on in this plot? ... It looks like there are two levels of shading?

Indeed -- that module has 12 cells per column and is 10% shaded, so the bottom row of cells is fully shaded and the second row is only partially shaded. Probably worth an explanation in the text.

PS thanks for these wonderful and extensive comments! I will go through them tonight and make updates.

nrow = cells_per_string//2
nrow_full_shade = int(shaded_fraction * nrow)
# find the fraction of shade in the border row
partial_shade_fraction = 1 - (shaded_fraction * nrow - nrow_full_shade)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you might like the numpy.divmod returns the floor-like quotient and remainder, altho maybe not here?

nrow_full_shade, partial_shade_fraction = np.divmod(shaded_fraction, 1/nrow)
partial_shade_fraction  = 1 - nrow*partial_shade_fraction 

@kandersolar
Copy link
Member Author

Thanks again for the reviews @mikofski and @cwhanse. I very much apologize for this bypass diode fiasco -- I was focused on max power only and since the shading was uniform, I didn't bother including them. Had I known that it would prompt such a strong reverse-bias review response, I'd have certainly included them from the start 😉

I think I've addressed most of the points, but I'm uncertain about these two:

  • thermal voltage definition: this is beyond my depth. Happy to do whatever someone else decides is a good idea. I notice I define it at 25C and then run models at 40C anyway.
  • singlediode.bishop88_i_from_v vs singlediode.bishop88 -- is there a technical reason to prefer one to the other? Or a pedagogical reason?

I'll fix the lint issues on the next update.

@cwhanse
Copy link
Member

cwhanse commented May 20, 2020

thermal voltage definition: this is beyond my depth. Happy to do whatever someone else decides is a good idea. I notice I define it at 25C and then run models at 40C anyway.

I think it's a good idea to state the cell temperature you assume and then calculate the thermal voltage rather than assume a thermal voltage.

singlediode.bishop88_i_from_v vs singlediode.bishop88 -- is there a technical reason to prefer one to the other? Or a pedagogical reason?

singlediode.bishop88_i_from_v lets you specify the voltages in the IV curve. singlediode.bishop88 asks you to provide diode_voltage as input, from which it calculates voltages for the IV curve. If trying to compare a model to measurement, bishop88_i_from_v is the better choice. Niether one is guaranteed to have a point at maximum power. Hence bishop88_mpp.

@kandersolar
Copy link
Member Author

@cwhanse I tried out switching to singlediode.bishop88_i_from_v for this example. It did work, but dramatically increased the runtime -- it's a couple of orders of magnitude slower than bishop88 and it really adds up from the repeated calls to build the heatmap matrix. I think it would be an annoyance for documentation builds. Ok to leave it as bishop88? Alternatively we could remove the heatmap section, or just do a single row of the matrix instead.

@mikofski
Copy link
Member

mikofski commented May 22, 2020

couple of orders of magnitude slower than bishop88

bishop88_i_from_v uses Brent or Newton to reverse each current in the sequence from given voltage implicitly, which would mean several calculations per point versus bishop88 which explicitly calculate both current and voltage using vector math from the given array of diode voltages.

Newton will be a little faster than Brent because it is vectorised (operates on arrays), whereas Brent loops over the sequence.

There is an unspoken goal to replace the Python-loop Brent method with a Cythonized no-GIL method using scipy.optimize.cython_optimize which should improve the runtime performance of Brent, but I never did it. There are some examples here in this gist Jupyter notebooks

@cwhanse
Copy link
Member

cwhanse commented May 22, 2020

Ok to leave it as bishop88?

Yes. Too bad that the runtime hit is that large. I do think that specifying cell rather than diode voltage is more familiar to most users, which is why I suggested using bishop88_i_from_v for this example.

@mikofski
Copy link
Member

mikofski commented May 22, 2020

Yes. Too bad that the runtime hit is that large.

Unless someone else wants to make a stab at it, I will try to make a PR for the cythonized version of Brent soonish. It should greatly improve the performance, benchmarks were between 10-100x faster than numpy.vectorize which is really no better than a for-loop, whereas scipy.optimize.cython_optimize with nogil can be compiled to use prange.

@kandersolar
Copy link
Member Author

@mikofski FYI I was actually using the newton method because brentq doesn't work with reverse bias. Per the bishop88_i_from_v docstring:

method (str, default 'newton') – Either 'newton' or 'brentq'. ‘’method’’ must be 'newton' if breakdown_factor is not 0.

I'm not against speeding up brentq, just wanted to point out that I don't think it would help this example.

@wholmgren wholmgren added this to the 0.8.0 milestone Jul 17, 2020
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kanderso-nrel for the contribution and @cwhanse and @mikofski for the detailed reviews. It looks like all of the concerns were addressed and I'm going to take the lack of further comment as approval and merge it.

@wholmgren wholmgren merged commit c67c9ec into pvlib:master Jul 17, 2020
@kandersolar kandersolar deleted the partial_shading_example branch July 18, 2020 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants