Skip to content

Ideal PV devices are not supported #324

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

Closed
markcampanelli opened this issue May 11, 2017 · 15 comments
Closed

Ideal PV devices are not supported #324

markcampanelli opened this issue May 11, 2017 · 15 comments
Milestone

Comments

@markcampanelli
Copy link
Contributor

markcampanelli commented May 11, 2017

One cannot define resistance_series = 0 or resistance_shunt = numpy.inf as "ideal" device parameters in pvlib.pvsystem.singlediode(). See the error below for example.

Is this feature valued enough for me to make a PR that adds this capability?

Input:

import sys
print("Python version: %s.%s.%s" % sys.version_info[:3])

import numpy as np
print("Numpy version: " + np.version.version)

import scipy as sp
print("Scipy version: " + sp.version.version)

import pvlib

i_ph_A = 2.13643772
i_rs_A = 2.10905501e-05
n_mod_V = 9.91331584
r_s_Ohm = 3.63866316
r_s_Ohm = 0.
g_p_Mho = 2.45880685e-03

pvlib.pvsystem.singlediode(i_ph_A, i_rs_A, r_s_Ohm, 1./g_p_Mho, n_mod_V)

with output:

Python version: 3.6.1
Numpy version: 1.12.1
Scipy version: 0.19.0
/Users/markcampanelli/anaconda3/lib/python3.6/site-packages/pvlib/pvsystem.py:1943: RuntimeWarning: divide by zero encountered in true_divide
  I = -V/(Rs + Rsh) - (nNsVth/Rs)*lambertwterm + Rsh*(IL + I0)/(Rs + Rsh)
/Users/markcampanelli/anaconda3/lib/python3.6/site-packages/pvlib/pvsystem.py:1943: RuntimeWarning: invalid value encountered in double_scalars
  I = -V/(Rs + Rsh) - (nNsVth/Rs)*lambertwterm + Rsh*(IL + I0)/(Rs + Rsh)
Out[1]:
OrderedDict([('i_sc', nan),
             ('v_oc', 112.87971015555131),
             ('i_mp', nan),
             ('v_mp', 0.02227117920245893),
             ('p_mp', nan),
             ('i_x', nan),
             ('i_xx', nan)])
@markcampanelli
Copy link
Contributor Author

Update: Sufficiently large values of resistance_shunt cause numerical issues as well. I'm not sure about the computational overhead to detect large Rsh issues and then possibly swap in the infinite shunt resistance approximation as appropriate.

Very small, yet strictly positive, resistance_series values seem to be better behaved in pvlib.pvsystem.singlediode(), which appears to rely heavily on pvlib.pvsystem.i_from_v(). I have not explored the "edge" behavior of pvlib.pvsystem.v_from_i(), but that is apparently called in pvlib.pvsystem.singlediode() to get Voc.

My concern here is that sometimes parameter fitting routines return resistance values that are at/near these edge cases.

@cwhanse
Copy link
Member

cwhanse commented May 11, 2017 via email

@adriesse
Copy link
Member

Solving the single diode equation is a lot easier when there is an infinite shunt resistance. I would suggest implementing a simpler solver to handle that case, if the general solver has trouble.

@wholmgren
Copy link
Member

I agree this would be a useful feature. A threshold seems reasonable.

Also relevant: #298, #268, #266, #260. We're waiting on a test case for #298.

@markcampanelli
Copy link
Contributor Author

markcampanelli commented May 11, 2017 via email

@wholmgren wholmgren added this to the 0.4.6 milestone May 30, 2017
@markcampanelli
Copy link
Contributor Author

I haven't forgotten about this. I think I may have a new solution here that covers both the typical and edge cases cleanly, efficiently, flexibly with respect to various input dimensions, and with surprisingly little code (leveraging numpy's ufunc's). My only question is how well the new algorithm converges for devices with very high series resistance (in the current computation given voltage) and/or low shunt resistance (in the voltage computation given current).

At the risk of appearing lazy here, can anyone point me to the specific unit tests around computing I-V curves?

@wholmgren
Copy link
Member

These are probably the most relevant:

https://github.com/pvlib/pvlib-python/blob/master/pvlib/test/test_pvsystem.py#L357-L379

There are some other tests in test_pvsystem.py that calculate these parameters, but they don't push the algorithm to its limits.

@markcampanelli
Copy link
Contributor Author

@wholmgren Thanks for the unit test ref's. I need some guidance on how to set up my local fork to allow me to run unit tests with pytest. Currently, I use pip to do a re-install of my local fork before I can run the unit tests against any code change using pytest pvlib/test/test_pvsystem.py from the root of my fork. This is far from ideal. How do I run unit tests when pvlib-python is not pip installed?

@wholmgren
Copy link
Member

Just to confirm: you ran pip install -e . from your cloned pvlib-python directory? As far as I know, that's the right thing to do. If you want to separate a clean pvlib from your development pvlib, then you probably should use conda environments or python's own virtual environments. I could have misunderstood something, though.

@markcampanelli
Copy link
Contributor Author

markcampanelli commented Jun 8, 2017 via email

@markcampanelli
Copy link
Contributor Author

@wholmgren pvlib.pvsystem.i_from_v() indicates that it returns "current : np.array". However, test_i_from_v() takes python scalars as input arguments and returns a numpy.float64, not a numpy.ndarray. Is numpy.float64 considered a subtype of np.array? More generally, what is the difference between numpy.array and numpy.ndarray?

@wholmgren
Copy link
Member

The numpy array type is ndarray, not array. The array function creates ndarray objects. So, the i_from_v docstring should read "current: np.ndarray or np.float64"

numpy.float64 is not a subtype of np.ndarray:

>>> isinstance(np.float64(5), np.ndarray)
False

@markcampanelli
Copy link
Contributor Author

Thanks for clarifying. These types behave a bit strange under the simplest of operations:

print(type(np.array(1.)))
print(type(np.array(1.) + np.array(1.)))

returns

<class 'numpy.ndarray'>
<class 'numpy.float64'>

@wholmgren
Copy link
Member

Yeah, I've never fully understood the type or broadcasting rules for numpy scalar or scalar-like arrays. The important thing for our function is that if you put scalars in, you get a scalar-like thing out, and if you put arrays in, you get arrays out. Other pvlib functions also guarantee that if you put a Series in, you get a series out, but this one does not.

@markcampanelli
Copy link
Contributor Author

Draft PR created: #340

@wholmgren wholmgren modified the milestones: 0.6.0, 0.4.6 Aug 7, 2017
wholmgren pushed a commit that referenced this issue Sep 29, 2017
* Add draft i_from_v_alt function with tests

* Better comments and more explicit typing

* Use transform from shunt resistance to shunt conductance

* Add v_from_i_alt() with initial tests and use np.where

* Use test fixtures

* Add @requires_scipy to test fixtures

* More current_sum_at_diode_node() tests and using fixture

* Naming, documentation, and formatting

* Deprecate replaced functions and flake8

* Add release documentation and flake8 again

* Replace deprecated function usages and update test_singlediode_series_ivcurve

* Conform to existing API

* Run flake8

* Implement some code quality suggestions

* Remove extraneous print statements

* Better docstrings

* Fix parameter ranges in docstrings

* Add test that overflows lambertw arg

* Add test for mixed solution types logic

* Use broadcast_arrays for cleaner code

* One more simplification

* Better use of broadcast_arrays
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants