-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Safely calculate dni from dhi and ghi #250
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
Changes from 32 commits
7d54462
b5994c6
abf02af
b9f6365
89fea56
21f5c5f
a2b6413
34865e5
8fd63b8
a46cdfa
98b6fe7
d3918ff
1959ed0
f1b7030
38343cd
35a95b4
e4e6139
3bbaddd
65f7d27
58df995
6a37e05
7994f28
1ef88a4
4bdb7ca
65a9261
e74faa9
0846db4
6505f4f
46b42a3
f3bd094
e1f5f58
9949661
4398017
aa82e57
0f5f113
ac98688
60a922c
8c05db1
2f08557
9df766f
e7a43df
d07ab49
2c253cb
e410d6c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2048,3 +2048,70 @@ def _get_dirint_coeffs(): | |
[0.743440, 0.592190, 0.603060, 0.316930, 0.794390]] | ||
|
||
return coeffs[1:, 1:, :, :] | ||
|
||
|
||
def dni(ghi, dhi, zenith, clearsky_dni=None, clearsky_tolerance=1, | ||
upper_cutoff_zenith=88.0, lower_cutoff_zenith=80.0, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no kwargs here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
""" | ||
Determine DNI from GHI and DHI. | ||
|
||
When calculating the DNI from GHI and DHI the calculated DNI may be | ||
unreasonably high or negative for zenith angles close to 90 degrees | ||
(sunrise/sunset transitions). This function identifies unreasonable DNI | ||
values and sets them to NaN. If the clearsky DNI is given unreasonably high | ||
values are cut off. | ||
|
||
Parameters | ||
---------- | ||
ghi : Series | ||
Global horizontal irradiance. | ||
|
||
dhi : Series | ||
Diffuse horizontal irradiance. | ||
|
||
zenith : Series | ||
True (not refraction-corrected) zenith angles in decimal | ||
degrees. Angles must be >=0 and <=180. | ||
|
||
clearsky_dni : None or Series | ||
Clearsky direct normal irradiance. Default: None. | ||
|
||
clearsky_tolerance : float | ||
If 'clearsky_dni' is given this parameter can be used to allow a | ||
tolerance by how much the calculated DNI value can be greater than | ||
the clearsky value before it is identified as an unreasonable value. | ||
Default: 1. | ||
|
||
upper_cutoff_zenith : float | ||
Zenith angle above which nonzero DNI values are set to NaN. | ||
Default: 88. | ||
|
||
lower_cutoff_zenith : float | ||
Zenith angle above which DNI values greater the clearsky DNI (plus | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "plus" should be "times" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
allowed tolerance) are corrected. Only applies if 'clearsky_dni' is not | ||
None. Default: 80. | ||
|
||
Returns | ||
------- | ||
dni : Series | ||
The modeled direct normal irradiance. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no extra space needed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
""" | ||
|
||
# calculate DNI | ||
dni_tmp = (ghi - dhi) / tools.cosd(zenith) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do you need dni_tmp and dni? maybe a holdover from a previous implementation? |
||
dni = dni_tmp.copy() | ||
|
||
# cutoff negative values | ||
dni[dni < 0] = float('nan') | ||
|
||
# set non-zero DNI values for zenith angles >= upper_cutoff_zenith to NaN | ||
dni[(zenith >= upper_cutoff_zenith) & (dni != 0)] = float('nan') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The doc string suggested to me that the comparison should be strictly greater than, rather than greater than or equal to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would that work and be correct English?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Zenith angles greater than or equal to upper_cutoff_zenith will be set to NaN. |
||
|
||
# correct DNI values for zenith angles between lower_cutoff_zenith and | ||
# upper_cutoff_zenith that are greater than the clearsky tolerance | ||
if clearsky_dni is not None: | ||
dni[(zenith >= lower_cutoff_zenith) & (zenith <= upper_cutoff_zenith) & | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above regarding comparison type |
||
(dni > (clearsky_dni * clearsky_tolerance))] = (clearsky_dni * | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're doing the same calculation twice, so it may be slightly faster and slightly more readable if you define and use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
clearsky_tolerance) | ||
return dni |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -623,7 +623,7 @@ def effective_irradiance_model(self): | |
fd*self.total_irrad['poa_diffuse']) | ||
return self | ||
|
||
def complete_irradiance(self, times=None, weather=None): | ||
def complete_irradiance(self, times=None, weather=None, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. as discussed elsewhere, probably no kwargs here, too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
""" | ||
Determine the missing irradiation columns. Only two of the | ||
following data columns (dni, ghi, dhi) are needed to calculate | ||
|
@@ -676,18 +676,23 @@ def complete_irradiance(self, times=None, weather=None): | |
"Results can be too high or negative.\n" + | ||
"Help to improve this function on github:\n" + | ||
"https://github.com/pvlib/pvlib-python \n") | ||
warnings.warn(wrn_txt, UserWarning) | ||
|
||
if {'ghi', 'dhi'} <= icolumns and 'dni' not in icolumns: | ||
logging.debug('Estimate dni from ghi and dhi') | ||
self.weather.loc[:, 'dni'] = ( | ||
(self.weather.loc[:, 'ghi'] - self.weather.loc[:, 'dhi']) / | ||
tools.cosd(self.solar_position.loc[:, 'zenith'])) | ||
self.weather.loc[:, 'dni'] = pvlib.irradiance.dni( | ||
self.weather.loc[:, 'ghi'], self.weather.loc[:, 'dhi'], | ||
self.solar_position.zenith, | ||
clearsky_dni=kwargs.get('clearsky_dni', | ||
self.location.get_clearsky(times).dni), | ||
clearsky_tolerance=kwargs.get('clearsky_tolerance', 1)) | ||
elif {'dni', 'dhi'} <= icolumns and 'ghi' not in icolumns: | ||
warnings.warn(wrn_txt, UserWarning) | ||
logging.debug('Estimate ghi from dni and dhi') | ||
self.weather.loc[:, 'ghi'] = ( | ||
self.weather.dni * tools.cosd(self.solar_position.zenith) + | ||
self.weather.dhi) | ||
elif {'dni', 'ghi'} <= icolumns and 'dhi' not in icolumns: | ||
warnings.warn(wrn_txt, UserWarning) | ||
logging.debug('Estimate dhi from dni and ghi') | ||
self.weather.loc[:, 'dhi'] = ( | ||
self.weather.ghi - self.weather.dni * | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would we be better off with a more descriptive name?
dni_from_components
? I'm not sure.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find the name dni_from_components a bit confusing since components could mean different things. Maybe we just name it after what it does
dni_from_ghi_and_dhi
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to
aoi
we could keepdni
, but I likeget_dni
orcalculate_dni
definitely better thandni_from_something
. We haven't used it so far and in my opinion the parameters should be part of the API documentation and not of the name.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems that
dni
is moderately acceptable to all.