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

updated first_solar_spectral_correction within atmosphere.py #208

Merged
merged 3 commits into from
Jul 11, 2016

Conversation

MLEEFS
Copy link
Contributor

@MLEEFS MLEEFS commented Jul 7, 2016

I updated first_solar_spectral_correction within atmosphere.py to account for changes in model that are reflected in the IEEE PVSC 43 publication. I also added data flags/filters in order to make sure that the function doesn't "blow up" when given extraordinary inputs.

Minimum precipitable water floor is set to 0.1cm. This presents the function from going toward -Inf as precipitable water goes to toward zero.

An absolute airmass ceiling is placed at AM = 10. This prevents function from destabilizing at very high AM. AM > 10 occur very close to sunset/sunrise when irradiance levels are very low.

Warning flags are now printed if Pwat is really high (above 8 cm), or if absolute air mass is very low (below .58)

account for changes in model that are reflected in the IEEE PVSC 43 publication
@wholmgren
Copy link
Member

Thanks, @MLEEFS. Looks good, just a few simple things...

It's best to replace the print statements with warnings.warn. This way, users can silence the warnings if they want to. I am ambivalent about input warnings for pvlib python, but if you want to use them, that's the way to do it.

You should also replace the limit setting with the np.maximum and np.minimum functions. For example:

pw = np.maximum(pw, 0.1)

These numpy functions should be much faster than the list comprehensions and I think they're more readable.

I'm a little surprised to see that the tests still passed without any modification, so your changes to the coefficients must amount to pretty small changes in output over the tested range (1-5 cm and 1-5 airmasses).

…porate recommendations from wholmgren. I replaced list comprehensions with numpy functions, and replaced print statements with warnings
@MLEEFS
Copy link
Contributor Author

MLEEFS commented Jul 8, 2016

Hi @wholmgren ,

I made the changes you requested. Can you confirm that they have been made? I am still getting used to the mechanics of GIT.

Models for PV Module Performance. National Renewable Energy
Laboratory, 2014. http://www.nrel.gov/docs/fy14osti/61610.pdf
"""

import pdb
Copy link
Member

Choose a reason for hiding this comment

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

You'll need to delete these pdb lines. They prevent the test suite from completing.

@@ -435,6 +474,6 @@ def first_solar_spectral_correction(pw, airmass_absolute, module_type=None,
AMa = airmass_absolute
modifier = (
coeff[0] + coeff[1]*AMa + coeff[2]*pw + coeff[3]*np.sqrt(AMa) +
+ coeff[4]*np.sqrt(pw) + coeff[5]*AMa/pw)
+ coeff[4]*np.sqrt(pw) + coeff[5]*AMa/np.sqrt(pw))
Copy link
Member

Choose a reason for hiding this comment

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

looks like I made a mistake when I first ported this and put two + signs here (one on each line). Let's keep only the first one.

@MLEEFS
Copy link
Contributor Author

MLEEFS commented Jul 8, 2016

the "bonus" lines of code should be removed

@wholmgren
Copy link
Member

Thanks @MLEEFS. merging...

@wholmgren wholmgren merged commit 58d77ea into pvlib:master Jul 11, 2016
@MLEEFS MLEEFS deleted the new_spectral branch July 11, 2016 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants