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

Suggestion to change the default solar constant in get_extra_radiation() #2099

Open
kurt-rhee opened this issue Jun 20, 2024 · 9 comments
Open

Comments

@kurt-rhee
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The default solar constant in pvlib's get_extra_radiation function is 1366.1. This default contradicts with the recommended solar constant in the default model's reference (Spencer).

Describe the solution you'd like
I would like to propose changing the default to 1360.8 per the references provided above.

Describe alternatives you've considered

  • We could leave the current value as the default and provide some information in the docstring as to why the default value and the values assumed in the reference materials are different.
@echedey-ls
Copy link
Contributor

We could also make the solar constant optional, then default to the one recommended by each model? A bit of over-engineering for such small differences, but possibly the best approach to be as accurate as possible with the references.

@adriesse
Copy link
Member

Many (all?) models were developed with specific values, so it's probably best if they default to those.

@kurt-rhee
Copy link
Contributor Author

kurt-rhee commented Jun 21, 2024

Agreed with @echedey-ls and @adriesse that each model should default to the values that they were developed with.

Reading through the provided references in the docstring here are the provided values for solar constant. Note that some of the references do not agree with each other.

[1] pg. 13, 1366.1 for Spencer model
[1] pg. 13, 1367.7 for ASCE model
[2] pg. 1, 1361.0 for Spencer model
[3] pg. 11, 136mW/cm^2 = 1360 for Unknown model
[4] unable to find a free version of this reference
[5] 4.92 MJ/m^2h = 1366.67 (PG 23)

I was unable to match references for pyephem and NREL methods, but I also didn't look very hard.

@kurt-rhee
Copy link
Contributor Author

I would also like to propose that we note which reference corresponds with which model choice. Does anybody know if there are similar patterns elsewhere in the project that I can copy?

@cwhanse
Copy link
Member

cwhanse commented Jun 21, 2024

Before we get too far, which modeling steps involve the solar constant? And, is it a problem if different values are used at each step?

@kurt-rhee
Copy link
Contributor Author

I wouldn't say that I am an expert on all of the modeling steps, but my understanding is that solar_constant only gets used to calculate the extraterrestrial DNI and then the models further down the model chain use extraterrestrial DNI as an input.

A quick search of pvlib though shows that there are two places where the term solar_constant is used:

  1. In the calculation of extraterrestrial DNI as expected.
  2. In the calculation of bird simple spectral model

Reference for 2:
https://pvlib-python.readthedocs.io/en/stable/_modules/pvlib/irradiance.html?highlight=solar_constant

    # ET spectral irradiance correction for earth-sun distance seasonality.
    # Note that we only want the distance correction coefficient, so set
    # solar_constant=1:
    earth_sun_distance_correction = \
        pvlib.irradiance.get_extra_radiation(dayofyear, method='spencer',
                                             solar_constant=1)  # Eq 2-2, 2-3

My thinking is that since, we are proposing only to change the default by model, these explicit over-rides will still be available for people to use.

@echedey-ls
Copy link
Contributor

echedey-ls commented Jun 24, 2024

Ref. [4], pp. 5-6 states:

A value of Gsc of 1367 W/m2 (1.960 cal/cm2 min, 433 Btu/ft2 h, or 4.921 MJ/m2 h) is used in this book.

https://breadl.org/d/275235 use mirror link no. 1 (IDK how much uptime do this pages have)

@adriesse
Copy link
Member

It would make a nice student project to research this topic more deeply present a summary report.

@echedey-ls
Copy link
Contributor

I have just seen this issue is a duplicate of #1566

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

No branches or pull requests

4 participants