Incidence Angle Modifier zero instead of np.nan (#338) #339

jkfm · 2017-06-14T09:00:02Z

This PR changes the three IAM-models to return 0 instead of np.nan for aoi >= 90 (ashraeiam, physicaliam) and <0 (sapm_aoi_loss).

I also updated the tests to account for the changes. The test for physicaliam now shows, that angles of -90 and 90 return 0, whereas an angle of 0 returns 1, in accordance to the model description in De Soto et al..

Comments are welcome!

…ulation of tau0

adriesse · 2017-06-14T14:26:36Z

Could we have the identical code in each of the three functions to do the same thing? That is, to deal with the negative values and the larger than 90 angles? I don't know whether np.maximum, np.where or np.clip is better, but I would lean toward clip.

The SAPM model doesn't like negative angles, the others are fine because of the cosine. That should be handled consistently as well.

wholmgren

Thanks @jkfm. Can you also make sure that the tests assert than the functions preserve nan values? I think they currently do, but this will prevent someone from inadvertently adding e.g. fillna to the functions in the future.

wholmgren · 2017-06-14T15:15:39Z

pvlib/pvsystem.py

@@ -811,6 +812,10 @@ def physicaliam(aoi, n=1.526, K=4., L=0.002):
    spa
    ashraeiam
    '''
+    zeroang = 1e-06
+
+    aoi = np.where(aoi == 0, zeroang, aoi)


Why did you add this line? I understand what it does, but not why we need it now when we didn't previously.

It's not clear to me that we actually need zeroang at all. I think its only purpose is to avoid infs, but maybe those be more cleanly handled with np.where or similar.

As I understood it, if the input angle is 0, tau will be nan, which is why we substitute it with zeroang.
As iam is defined as tau(theta) / tau(0°), we need zeroang anyway to calculate tau(0°). That's why I moved the variable definition to the beginning of the function.

I guess we could define tau0 as a constant in the function and handle aoi == 0 with np.where after the calculations are done, but I feel like the current function is more intuitive to understand.

We should keep zeroang and for input theta < zeroang, physicaliam should return tau0. Passing theta=0 through will return nan rather than zero. Line 821 computes tau with the leading factor exp(-KL/cos(theta)) which is why we should return tau0 for small theta.

tau0 is computed awkwardly. I'd replace line 829 - 833 with the limit value which we should have implemented in Matlab:
tau0 = exp(-KL)*(1 - ((1-n)/(1+n))^2)
We used zeroang instead in Matlab which is probably why its in pvlib-python

ok, now I see that you've changed the test so that an input of 0 returns 1 instead of np.nan. I should have caught this when I wrote the test.

I am surprised that the model would not work for an angle of exactly zero degrees. Could this have something to do with the equation discrepancy mentioned in the comments further above?

I don't think it has to do with the function discrepancy. The unmodified function for theta_r is not defined for angles > ~40°.

The tau(theta)-function in turn simply has a discontinuity for theta = 0. It seems this was ignored in the paper...

wholmgren · 2017-06-14T15:19:01Z

pvlib/pvsystem.py

@@ -732,7 +732,7 @@ def ashraeiam(aoi, b=0.05):

    iam = 1 - b*((1/np.cos(np.radians(aoi)) - 1))

-    iam = np.where(np.abs(aoi) >= 90, np.nan, iam)
+    iam = np.where(np.abs(aoi) >= 90, 0, iam)
    iam = np.maximum(0, iam)


is the np.maximum line still necessary?

I think it is, because for angles close to +/-90° the function would return a negative value for the iam (the intercept for b=0.05 is at 87.21°: https://www.desmos.com/calculator/y8wvzrx3b9).

I agree, the np.maximum line is needed

wholmgren · 2017-06-14T15:30:08Z

@adriesse brings up a separate but closely related point regarding the meaning of negative values of aoi in pvlib. I tend to agree that the aoi loss functions should all start with something like aoi = np.abs(aoi), but that's open for discussion. I thought that we had previously discussed this elsewhere, but I can't find the discussion. If we're going to change it, now is the time.

cwhanse · 2017-06-14T16:06:51Z

If there are negative values for AOI, that indicates a problem elsewhere. I’m not in favor of using abs(aoi) to address it.

…input angles close to 90°

wholmgren

Thanks @jkfm.

If we're not going to treat negative aoi as being equivalent to positive aoi, then my preference is for nans for both AOI < 0 and AOI > 90. Another option is 0 for either or both of those ranges.

In any case, we should update the Variables and Symbols page with the allowable AOI domain. http://pvlib-python.readthedocs.io/en/latest/variables_style_rules.html

wholmgren · 2017-06-30T17:35:21Z

pvlib/pvsystem.py

@@ -811,6 +812,10 @@ def physicaliam(aoi, n=1.526, K=4., L=0.002):
    spa
    ashraeiam
    '''
+    zeroang = 1e-06
+
+    aoi = np.where(aoi == 0, zeroang, aoi)


ok, now I see that you've changed the test so that an input of 0 returns 1 instead of np.nan. I should have caught this when I wrote the test.

cwhanse · 2017-06-30T21:04:56Z

I vote for IAM=0 where AOI>=90, which is consistent with its definition and its use in the modeling process. Where AOI=0 (or very close to 0) in the physical IAM model the formula needs to switch to the limit expression in my earlier comment. That switch should be the use of zeroang.

…

Sent from my iPhone

jkfm · 2017-07-03T13:18:45Z

I would also prefer IAM to return zero for angles outside -90/90 degrees.

cwhanse · 2017-06-16T14:06:53Z

pvlib/pvsystem.py

@@ -732,7 +732,7 @@ def ashraeiam(aoi, b=0.05):

    iam = 1 - b*((1/np.cos(np.radians(aoi)) - 1))

-    iam = np.where(np.abs(aoi) >= 90, np.nan, iam)
+    iam = np.where(np.abs(aoi) >= 90, 0, iam)
    iam = np.maximum(0, iam)


I agree, the np.maximum line is needed

cwhanse · 2017-06-16T14:22:32Z

pvlib/pvsystem.py

@@ -811,6 +812,10 @@ def physicaliam(aoi, n=1.526, K=4., L=0.002):
    spa
    ashraeiam
    '''
+    zeroang = 1e-06
+
+    aoi = np.where(aoi == 0, zeroang, aoi)


We should keep zeroang and for input theta < zeroang, physicaliam should return tau0. Passing theta=0 through will return nan rather than zero. Line 821 computes tau with the leading factor exp(-KL/cos(theta)) which is why we should return tau0 for small theta.

tau0 is computed awkwardly. I'd replace line 829 - 833 with the limit value which we should have implemented in Matlab:
tau0 = exp(-KL)*(1 - ((1-n)/(1+n))^2)
We used zeroang instead in Matlab which is probably why its in pvlib-python

cwhanse · 2017-07-03T19:09:24Z

pvlib/pvsystem.py

@@ -831,7 +834,7 @@ def physicaliam(aoi, n=1.526, K=4., L=0.002):



Use
tau0 = (np.exp(- 1.0 * (K*L)) * (1 - (1 - n)^2/(1 + n)^2)
instead of the expression on line 826. I'd switch to tau0 when thetar_deg<zeroing.

I agree with your suggestions, especially after having consulted my D&B book. I would suggest calculating reflectance and absorption separately before combining them in order to clarify the code.

I think I applied both your suggestions correctly. Could you take a look?

It wasn't quite correct, so I thought it might be easier to communicate it this way:

# angle of refraction thetar_deg = tools.asind(tools.sind(aoi) / n) # reflectance and transmittance for normal incidence light rho_zero = ((1-n) / (1+n)) ** 2 tau_zero = np.exp(-K*L) # reflectance for parallel and perpendicular polarized light rho_para = (tools.tand(thetar_deg - aoi) / tools.tand(thetar_deg + aoi)) ** 2 rho_perp = (tools.sind(thetar_deg - aoi) / tools.sind(thetar_deg + aoi)) ** 2 # transmittance for non-normal light tau = np.exp(-K*L / tools.cosd(thetar_deg)) # iam is ratio of non-normal to normal incidence transmitted light # after deducting the reflected portion of each iam = ((1 - (rho_para + rho_perp) / 2) / (1 - rho_zero) * tau / tau_zero ) # angles near zero produce nan, but iam is defined as one small_angle = 1e-06 iam = np.where(np.abs(aoi) < small_angle, 1.0, iam) # angles at 90 degrees can produce tiny negative values, which should be zero # this is a result of calculation precision rather than the physical model iam = np.where(iam < 0, 0, iam) # for light coming from behind the plane, none can enter the module iam = np.where(aoi > 90, 0, iam) # negative angles are not logical iam = np.where(aoi < 0, np.nan, iam)

Thanks for the detailed explanation! So does this refer to reference 2 in the Docstring (I don't have access so cannot check)?

Ref [1] simply describes what is in ref [2].

I implemented your suggestion except for setting negative angles to np.nan, to keep consistency with the other IAM functions for now...

Consistency has some appeal...

adriesse · 2017-07-06T08:15:10Z

Another observation: contrary to the statement in ref [1] the default value of 2 mm for the glass thickness is definitely at the thin extreme. The value I currently see most often is 3.2 mm. Previously I believe 4 mm was more common.

jkfm · 2017-07-18T07:38:25Z

I agree with the default value, but I could not find any sources on it. Do you think, we should change the default nevertheless?

…' into modelchain_zero_instead_of_npnan

adriesse · 2017-07-18T19:51:02Z

The source for the 2mm default has no sources either...

cwhanse · 2017-07-18T20:15:14Z

If it helps, the model output is essentially insensitive to the thickness parameter. You can double from 0.002 to 0.004 and the IAM changes very slightly.

For myself, I'd prefer the default to remain the value in the reference paper, simply for traceability back to literature. I do see merit in changing to 3.2mm, though.

wholmgren · 2017-07-31T18:40:17Z

I suggest keeping the default set to 2 mm. I'll merge tomorrow unless there are objections.

nextkaufmann added 4 commits June 14, 2017 09:22

changed np.nan to 0 in functions ashraeiam, physicaliam, sapm_aoi_loss

24d8fbc

updated docstrings

acacf3a

updated tests to account for changes in functions

6167d7f

replace 0-degree aoi in physicaliam with 1e-06, analogous to the calc…

d92f8fb

…ulation of tau0

wholmgren added this to the 0.5.0 milestone Jun 14, 2017

wholmgren added the api label Jun 14, 2017

wholmgren reviewed Jun 14, 2017

View reviewed changes

nextkaufmann added 3 commits June 16, 2017 09:10

updated tests to account for input angles of np.nan

fbbff59

updated test for ashraeiam to check for possible negative values for …

51340e2

…input angles close to 90°

updated docstring with info about nan-inputs resulting in nan.

990acf1

jkfm changed the title ~~Modelchain zero instead of npnan (#338)~~ Incidence Angle Modifier zero instead of np.nan (#338) Jun 16, 2017

wholmgren reviewed Jun 30, 2017

View reviewed changes

wholmgren mentioned this pull request Jun 30, 2017

add explicit tests for aoi and aoi projection functions #345

Merged

nextkaufmann added 2 commits July 3, 2017 15:10

updated variables-and-symbols-page with allowable range of aoi

3d3e8b5

changed whatsnew for v0.5

31461b1

cwhanse reviewed Jul 3, 2017

View reviewed changes

nextkaufmann and others added 4 commits July 4, 2017 17:12

applied @adriesse suggestion to split calculation of tau

28c5e17

Merge branch 'master' into modelchain_zero_instead_of_npnan

23e67e1

forgot to delete unused line

a607a10

typo :-(

a329a8a

cwhanse approved these changes Jul 4, 2017

View reviewed changes

nextkaufmann added 2 commits July 18, 2017 09:53

typo in variables_styles_rules

fd593a2

implemented code suggestion by @adriesse

6ca99af

Merge remote-tracking branch 'origin/modelchain_zero_instead_of_npnan…

a52dba0

…' into modelchain_zero_instead_of_npnan

wholmgren mentioned this pull request Jul 24, 2017

ModelChain+SingleAxisTracker aoi calculations are wrong, too easy to miscalculate AOI using SingleAxisTracker #351

Closed

wholmgren merged commit 3b0a86b into pvlib:master Aug 1, 2017

wholmgren mentioned this pull request Aug 1, 2017

Set negative values of AOI loss to 0 rather than np.nan #338

Closed

jkfm deleted the modelchain_zero_instead_of_npnan branch December 14, 2017 13:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Incidence Angle Modifier zero instead of np.nan (#338) #339

Incidence Angle Modifier zero instead of np.nan (#338) #339

jkfm commented Jun 14, 2017 •

edited

Loading

adriesse commented Jun 14, 2017

wholmgren left a comment

wholmgren Jun 14, 2017

jkfm Jun 16, 2017 •

edited

Loading

cwhanse Jun 16, 2017

wholmgren Jun 30, 2017

adriesse Jun 30, 2017

jkfm Jul 3, 2017 •

edited

Loading

wholmgren Jun 14, 2017

jkfm Jun 16, 2017

cwhanse Jun 16, 2017

wholmgren commented Jun 14, 2017

cwhanse commented Jun 14, 2017 via email •

edited by wholmgren

Loading

wholmgren left a comment •

edited

Loading

wholmgren Jun 30, 2017

cwhanse commented Jun 30, 2017 via email •

edited by wholmgren

Loading

jkfm commented Jul 3, 2017

cwhanse Jun 16, 2017

cwhanse Jun 16, 2017

cwhanse Jul 3, 2017

adriesse Jul 3, 2017

jkfm Jul 4, 2017

adriesse Jul 4, 2017

jkfm Jul 6, 2017

adriesse Jul 6, 2017

jkfm Jul 18, 2017

adriesse Jul 18, 2017

adriesse commented Jul 6, 2017

jkfm commented Jul 18, 2017

adriesse commented Jul 18, 2017

cwhanse commented Jul 18, 2017

wholmgren commented Jul 31, 2017

		@@ -831,7 +834,7 @@ def physicaliam(aoi, n=1.526, K=4., L=0.002):

Incidence Angle Modifier zero instead of np.nan (#338) #339

Incidence Angle Modifier zero instead of np.nan (#338) #339

Conversation

jkfm commented Jun 14, 2017 • edited Loading

adriesse commented Jun 14, 2017

wholmgren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkfm Jun 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkfm Jul 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wholmgren commented Jun 14, 2017

cwhanse commented Jun 14, 2017 via email • edited by wholmgren Loading

wholmgren left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwhanse commented Jun 30, 2017 via email • edited by wholmgren Loading

jkfm commented Jul 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adriesse commented Jul 6, 2017

jkfm commented Jul 18, 2017

adriesse commented Jul 18, 2017

cwhanse commented Jul 18, 2017

wholmgren commented Jul 31, 2017

jkfm commented Jun 14, 2017 •

edited

Loading

jkfm Jun 16, 2017 •

edited

Loading

jkfm Jul 3, 2017 •

edited

Loading

cwhanse commented Jun 14, 2017 via email •

edited by wholmgren

Loading

wholmgren left a comment •

edited

Loading

cwhanse commented Jun 30, 2017 via email •

edited by wholmgren

Loading