Skip to content

DIRINT and DIRINDEX discrepancies between 0.5.2 and 0.6.3 #738

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
cedricleroy opened this issue Jun 12, 2019 · 16 comments · Fixed by #760
Closed

DIRINT and DIRINDEX discrepancies between 0.5.2 and 0.6.3 #738

cedricleroy opened this issue Jun 12, 2019 · 16 comments · Fixed by #760

Comments

@cedricleroy
Copy link
Contributor

We are in the process of upgrading PVLib from v0.5.2 to v0.6.3, and caught some big changes in the DIRINT and DIRINDEX functions. Here are some plots ran from the same notebook with both versions:

0.5.2

image

image

image

image

0.6.3

The GHI clearsky (used by the DIRINDEX model) seems to be kind of shifted compared to 0.5.2:

image

image

image

DIRINT changed compared to 0.5.2 and DIRINDEX is way off:

image

Is this expected? Or are we doing anything wrong? It seems it is due to the changes in #400. Wanted to share this before digging further into the root cause.

Here is the notebook, and below are the data.

pvlib-data.csv.zip

@mikofski
Copy link
Member

maybe this is related to changes in the clear sky model in #459 which was part of v0.6.0?

@cwhanse
Copy link
Member

cwhanse commented Jun 12, 2019

@cedricleroy not expected to see that increase in DNI at high zenith/air mass. @mikofski maybe #459 but I think it's more likely due to the zenith angle limit.

@cedricleroy could you plot some intermediates from the dirindex function, outputs of these lines:

    dni_dirint = dirint(ghi, zenith, times, pressure=pressure,
                        use_delta_kt_prime=use_delta_kt_prime,
                        temp_dew=temp_dew, min_cos_zenith=min_cos_zenith,
                        max_zenith=max_zenith)

    dni_dirint_clearsky = dirint(ghi_clearsky, zenith, times,
                                 pressure=pressure,
                                 use_delta_kt_prime=use_delta_kt_prime,
                                 temp_dew=temp_dew,
                                 min_cos_zenith=min_cos_zenith,
                                 max_zenith=max_zenith)

I might get to some diagnostics later this week.

@cedricleroy
Copy link
Contributor Author

df['DNI_dirindex_dirint_ghi'] = pvlib.irradiance.dirint(df['GHI_measured'], solpos['zenith'], df.index)
df['DNI_dirindex_dirint_ghi_clearsky'] = pvlib.irradiance.dirint(cs['ghi'], solpos['zenith'], df.index)

v0.5.2

image

v0.6.3

image

@cedricleroy
Copy link
Contributor Author

@mikofski, @cwhanse: Confirmed that it has been introduced by #459. We don't have problems anymore when using the clear-sky model with perez_enhancement=True.

As the default is False, should expect users to be aware of that?

@mikofski
Copy link
Member

Another option might be to modify dirindex to use any clear sky as input - it's there a possibility for you to use simplified solis? TMY3 has aod and pwat

@cwhanse
Copy link
Member

cwhanse commented Jun 13, 2019

@mikofski I'd prefer not to do that in pvlib. The original reference for DIRINDEX builds in the Ineichen clearsky model. I don't think it's wrong to substitute a different clear sky model, but the result is not DIRINDEX as documented.

@cwhanse
Copy link
Member

cwhanse commented Jun 13, 2019

The paper behind DIRINDEX assumes the Ineichen model using the Perez enhancement. It appears that the enhancement was introduced as part of the DIRINDEX model, apparently to offset some systematic discrepancy. When the enhancement term is omitted then DIRINDEX suffers.

For the near term, @cedricleroy can move ahead by modifying his workflow. In pvlib, I see a few options:

  1. do nothing and point re-discoverers of this problem to this discussion. I vote no here.
  2. add a usage note to DIRINDEX docstring about the calculating clearsky_ghi and clearsky_dni inputs using Ineichen.
  3. bring calculation of the clearsky GHI into DIRINDEX. I would prefer this option. It removes the flexibility of substituting other clear sky models for Ineichen, but DIRINDEX wasn't developed independent of Ineichen, and there's no validation outside of Ineichen.

@cwhanse
Copy link
Member

cwhanse commented Jun 18, 2019

Any objections to #3 above, as the resolution path?

@cedricleroy
Copy link
Contributor Author

  1. makes sense to me, it is what I would have suggested.

@markcampanelli
Copy link
Contributor

@cedricleroy Does perez_enhancement=True give suspect results for you at higher latitudes? If yes, then it's not clear to me if resolution path #3 also solves this issue.

@wholmgren
Copy link
Member

I vote for 2. Bringing the clear sky calculation into this function would require major API changes.

@cwhanse
Copy link
Member

cwhanse commented Jun 19, 2019

I vote for 2. Bringing the clear sky calculation into this function would require major API changes.

How so? My take is that the signature for dirindex would look more like dirint. We would not expose perez_enhancement as an argument in dirindex, since that is part of the DIRINDEX model, as I interpret the paper.

Edit - I see your point now, we'd drop a few arguments but would need to add others for the embedded call to ineichen.

@cedricleroy
Copy link
Contributor Author

I think what @wholmgren means is that ghi_clearsky and dni_clearsky are mandatory input to dirindex. If they are calculated within the function (not inputs anymore), it breaks the API.
As a user, I personally prefer the API breaking than upgrading to a version which leads to bad results without notification. And since PVLib implements "as per paper", removing the inputs will be less ambiguous maybe.

@cedricleroy Does perez_enhancement=True give suspect results for you at higher latitudes? If yes, > then it's not clear to me if resolution path #3 also solves this issue.

Hey @markcampanelli. I am not aware of this issue. What is a high latitude? Are you experiencing this?

@mikofski
Copy link
Member

@cedricleroy for more info on Ineichen clear sky high latitude discrepencies, see my previous comment, pr #459, and issue #435. In a nutshell, Ineichen mornings and evenings have unphysical peaks at high altitudes due to an undocumented extra parameter that seems to have been added just for Perez's DIRINDEX model maybe?

@markcampanelli
Copy link
Contributor

@cedricleroy I can offer no further insight, other than I recognized the potential artifact after reading through the above referenced issue and pr.

@cwhanse
Copy link
Member

cwhanse commented Jun 24, 2019

due to an undocumented extra parameter that seems to have been added just for Perez's DIRINDEX model maybe?

That's the way I interpret the sequence of papers - the "Perez enhancement" appears in the paper describing DIRINDEX, and I'm of the point of view that the enhancement is intrinsic to the DIRINDEX model. To be fair it is documented but not emphasized.

@cwhanse cwhanse mentioned this issue Aug 2, 2019
6 tasks
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 a pull request may close this issue.

5 participants