Skip to content
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

Add a public constants module and include scipy as base requirement for pvlib #483

Closed
markcampanelli opened this issue Jun 16, 2018 · 11 comments · Fixed by #1617
Closed

Comments

@markcampanelli
Copy link
Contributor

pvlib should have a public constants python module so that all the different models share the same constants and so that users can see what constants pvlib is using and match them if/when needed.

For example, the data values in a gold dataset for the single-diode model depend on the choice of constants, see this PR. Note that scipy.constants contains many useful physical constants traceable to CODATA, as well as temperature conversion functions and the like.

We also have to make sure we distinguish between the same constant with different units, or agree to use only one version of each constant throughout the codebase.

I think we should wrap the scipy.constants and make scipy an import requirement for the "base" installation of pvlib. (This would also remove a lot of the test decorations and import checks around scipy, and scipy.special.lambertw is already needed for pvsystem.v_from_i and pvsystem.i_from_v.) Simple tests should be written to catch and audit when constants have changed.

@wholmgren
Copy link
Member

Thanks for making this issue @thunderfish24

We can make a more informed decision if we have a list of constants that we use in pvlib (or are likely to use in the future) and a list of the functions that require scipy.

What is the point of a pvlib.constants module if we add scipy as a base dependency? Why not from scipy.constants import Boltzmann where needed? Direct imports from scipy seem more straightforward to me.

I am not inclined to keep track of constants values in scipy. I think that is the user's job when deciding what version of scipy to install.

Can anyone comment on easy of scipy installations on Windows and microcomputers in 2018?

@mikofski
Copy link
Member

Wheels are available for Windows at the CheeseShop. I haven't installed them so perhaps I shouldn't comment. I hesitate to ask on the SciPy Dev mail list. But my guess is that most pvlib users are probably using Anaconda, and there's always Christoph Gohlke.

@wholmgren
Copy link
Member

Thanks @mikofski. I see the wheels are only ~30 MB, which is not so bad (and comparable to pvlib itself), so probably ok for microcomputers.

My biggest concern is making sure that a new user that knows some kind of integrated scientific programming environment (matlab, mathematica, igor, idl, etc) but little python and command line can install pvlib with as little trouble as possible.

What is the minimum version of scipy that we would require?

This may also require bumping the minimum numpy and pandas versions. Bumping numpy to 1.10 would allow us to remove another test decorator.

Which brings me to thinking about the python versions we test against (and thus semi-officially support), but that's probably best left for another issue.

@wholmgren
Copy link
Member

wholmgren commented Jun 17, 2018

Here's the list of pvlib functions and what they use from scipy:

  • clearsky.detect_clearsky: from scipy.linalg import hankel
  • clearsky.detect_clearsky: from scipy.optimize import minimize_scalar
  • pvsystem.v_from_i: from scipy.special import lambertw
  • pvsystem.i_from_f: from scipy.special import lambertw
  • solarposition.calc_time: import scipy.optimize as so ... so.brentq

Here are the constants:

  • calcparams_desoto: kb, celcius/kelvin conversion
  • sapm: q, kb, celcius/kelvin conversion

I'm guessing I'm missing something. Let me know and I'll add it to these lists so that they're all in one place.

@mikofski
Copy link
Member

@cwhanse
Copy link
Member

cwhanse commented Jun 18, 2018

Keep in mind, the units on the Boltzmann constant are different : sapm uses J/K, calcparams_desoto uses eV/K.

@markcampanelli
Copy link
Contributor Author

markcampanelli commented Jun 19, 2018

@wholmgren We might also consider these "default" values related to bandgaps: EgRef = 1.121 (eV) and dEgdT = -0.0002677 (eV/K) for x-Si, and the others for CdTe, CIGS, etc.

@cwhanse
Copy link
Member

cwhanse commented Jun 19, 2018

EgRef and dEgdT are model parameters specific to De Soto and CEC. I wouldn't put them in the constants module, although EgRef has a physical basis, its meaning in these models is tainted by the application of the c-Si value to other absorber types.

@mikofski
Copy link
Member

mikofski commented Sep 6, 2020

#1035 was merged. Anyone okay with either closing this or moving it to v0.9 like #825?

@wholmgren
Copy link
Member

@mikofski i think you meant 1035. Yes, let's push discussion of a pvlib constants module to another release.

@mikofski mikofski modified the milestones: 0.8.0, 0.9.0 Sep 6, 2020
@cwhanse
Copy link
Member

cwhanse commented Sep 6, 2020

Agree, CONSTANTS for v0.9

@kandersolar kandersolar modified the milestones: 0.10.0, 0.9.4 Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants