-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix2448 #2485
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
Open
OmarBahamida
wants to merge
3
commits into
pvlib:main
Choose a base branch
from
OmarBahamida:fix2448
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Fix2448 #2485
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,16 +22,23 @@ There is a convention on consistent variable names throughout the library: | |
|
||
aoi | ||
Angle of incidence. Angle between the surface normal vector and the | ||
vector pointing towards the sun’s center | ||
vector pointing towards the sun's center. Must be >=0 and <=180 degrees. | ||
When the sun is behind the surface, the value is >90 degrees. | ||
|
||
aoi_projection | ||
cos(aoi) | ||
cos(aoi). When the sun is behind the surface, the value is negative. | ||
For many uses, negative values must be set to zero. | ||
|
||
ape | ||
Average photon energy | ||
|
||
apparent_zenith | ||
Refraction-corrected solar zenith angle in degrees | ||
Refraction-corrected solar zenith angle in degrees. Must be >=0 and <=180. | ||
This angle accounts for atmospheric refraction effects. | ||
|
||
apparent_elevation | ||
Refraction-corrected solar elevation angle in degrees. Must be >=-90 and <=90. | ||
This is the complement of apparent_zenith (90 - apparent_zenith). | ||
|
||
bhi | ||
Beam/direct horizontal irradiance | ||
|
@@ -87,10 +94,10 @@ There is a convention on consistent variable names throughout the library: | |
Sandia Array Performance Model IV curve parameters | ||
|
||
latitude | ||
Latitude | ||
Latitude in decimal degrees. Positive north of equator, negative to south. | ||
|
||
longitude | ||
Longitude | ||
Longitude in decimal degrees. Positive east of prime meridian, negative to west. | ||
|
||
pac, ac | ||
AC power | ||
|
@@ -141,10 +148,14 @@ There is a convention on consistent variable names throughout the library: | |
Diode saturation current | ||
|
||
solar_azimuth | ||
Azimuth angle of the sun in degrees East of North | ||
Azimuth angle of the sun in degrees East of North. Must be >=0 and <=360. | ||
The convention is defined as degrees east of north (e.g. North = 0°, | ||
East = 90°, South = 180°, West = 270°). | ||
|
||
solar_zenith | ||
Zenith angle of the sun in degrees | ||
Zenith angle of the sun in degrees. Must be >=0 and <=180. | ||
This is the angle between the sun's rays and the vertical direction. | ||
This is the complement of :term:`solar_elevation` (90 - elevation). | ||
|
||
spectra | ||
spectra_components | ||
|
@@ -154,11 +165,14 @@ There is a convention on consistent variable names throughout the library: | |
is composed of direct and diffuse components. | ||
|
||
surface_azimuth | ||
Azimuth angle of the surface | ||
Azimuth angle of the surface in degrees East of North. Must be >=0 and <=360. | ||
The convention is defined as degrees east (clockwise) of north. This is pvlib's | ||
convention; other tools may use different conventions. For example, North = 0°, | ||
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. I suggest moving the sentence, "For example, North...", before the sentence, "This is pvlib's convention...". As it is, I interpret the example as an example of some other tool's convention, not as an example of pvlib's convention. |
||
East = 90°, South = 180°, West = 270°. | ||
|
||
surface_tilt | ||
Panel tilt from horizontal [°]. For example, a surface facing up = 0°, | ||
surface facing horizon = 90°. | ||
Panel tilt from horizontal [°]. Must be >=0 and <=180. | ||
For example, a surface facing up = 0°, surface facing horizon = 90°. | ||
|
||
temp_air | ||
Temperature of the air | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
# Simple pvlib demonstration script | ||
import pvlib | ||
import pandas as pd | ||
from datetime import datetime, timedelta | ||
import matplotlib.pyplot as plt | ||
|
||
# Create a location object for a specific site | ||
location = pvlib.location.Location( | ||
latitude=40.0, # New York City latitude | ||
longitude=-74.0, # New York City longitude | ||
tz='America/New_York', | ||
altitude=10 # meters above sea level | ||
) | ||
|
||
# Calculate solar position for a day | ||
date = datetime(2024, 3, 15) | ||
times = pd.date_range(date, date + timedelta(days=1), freq='1H', tz=location.tz) | ||
solpos = location.get_solarposition(times) | ||
|
||
# Plot solar position | ||
plt.figure(figsize=(10, 6)) | ||
plt.plot(solpos.index, solpos['elevation'], label='Elevation') | ||
plt.plot(solpos.index, solpos['azimuth'], label='Azimuth') | ||
plt.title('Solar Position for New York City on March 15, 2024') | ||
plt.xlabel('Time') | ||
plt.ylabel('Angle (degrees)') | ||
plt.legend() | ||
plt.grid(True) | ||
plt.show() | ||
|
||
# Calculate clear sky irradiance | ||
clearsky = location.get_clearsky(times) | ||
|
||
# Plot clear sky irradiance | ||
plt.figure(figsize=(10, 6)) | ||
plt.plot(clearsky.index, clearsky['ghi'], label='Global Horizontal Irradiance') | ||
plt.plot(clearsky.index, clearsky['dni'], label='Direct Normal Irradiance') | ||
plt.plot(clearsky.index, clearsky['dhi'], label='Diffuse Horizontal Irradiance') | ||
plt.title('Clear Sky Irradiance for New York City on March 15, 2024') | ||
plt.xlabel('Time') | ||
plt.ylabel('Irradiance (W/m²)') | ||
plt.legend() | ||
plt.grid(True) | ||
plt.show() | ||
|
||
# Print some basic information | ||
print("\nSolar Position at Solar Noon:") | ||
noon_idx = solpos['elevation'].idxmax() | ||
print(f"Time: {noon_idx}") | ||
print(f"Elevation: {solpos.loc[noon_idx, 'elevation']:.2f}°") | ||
print(f"Azimuth: {solpos.loc[noon_idx, 'azimuth']:.2f}°") | ||
|
||
print("\nMaximum Clear Sky Irradiance:") | ||
print(f"GHI: {clearsky['ghi'].max():.2f} W/m²") | ||
print(f"DNI: {clearsky['dni'].max():.2f} W/m²") | ||
print(f"DHI: {clearsky['dhi'].max():.2f} W/m²") |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'm mildly opposed to including this range description. I don't think that pvlib functions require that constraint. It is certainly not checked for the user. And I don't think that pvlib promises useful results if
aoi
falls in that range.Is the intent here to inform a user of the usual range for this quantity? If so, and we agree to add a statement about "typical" ranges, then I don't think we should use the verb "must".
@pvlib/pvlib-maintainer please weigh in.
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 agree with Cliff. I don’t think this constraint is necessary. A scan of the
irradiance.py
module yields the following functions withaoi
as an argument:poa_components
has a note that says if AOI is negative or >90° then components would be zero…pvlib-python/pvlib/irradiance.py
Line 500 in a27c686
… but this is incorrect according to the code. It will only be zero if < -90° or >90°
pvlib-python/pvlib/irradiance.py
Line 504 in a27c686
gti_dirint
has two subfunctions that depend on whether AOI is greater than or less than 90° which I think is a bug 🐞 and probable should read ifabs(AOI) < 90
pvlib-python/pvlib/irradiance.py
Line 2384 in a27c686
If not a bug 🐞 then it seems to me that AOI is assumed to be in the range of [0, 180] but then it’s still a a bug 🐞 because AFAICT this is never enforced
iam.py
also uses AOI for example in ASHRAE, it clearly limitsabs(AOI) < 90
pvlib-python/pvlib/iam.py
Line 85 in a27c686
Anyway, I suggest continue searching through the code, and find out what it actually says because it seems to me like there may be different assumptions
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.
@OmarBahamida to let this PR move ahead without getting really involved, let's leave out any statements about a range of values.