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

gti_dirint raises 'UnboundLocalError: local variable 'best_ghi' referenced before assignment' #1342

Open
kdebrab opened this issue Nov 24, 2021 · 6 comments · May be fixed by #2347
Open

gti_dirint raises 'UnboundLocalError: local variable 'best_ghi' referenced before assignment' #1342

kdebrab opened this issue Nov 24, 2021 · 6 comments · May be fixed by #2347
Labels

Comments

@kdebrab
Copy link
Contributor

kdebrab commented Nov 24, 2021

Describe the bug
irradiance.gti_dirint fails for surface faced North

To Reproduce

import pvlib
import pandas as pd
times = pd.date_range('2021-11-24', '2021-11-25', freq='h')
latitude, longitude, surface_tilt, surface_azimuth = 51.3, 4.56, 25, 360
solpos = pvlib.solarposition.get_solarposition(times, latitude, longitude)
solar_zenith, solar_azimuth = solpos['apparent_zenith'], solpos['azimuth']
ghi = pvlib.clearsky.simplified_solis(90 - solar_zenith)['ghi']
poa_global = ghi  # this is not correct, but doesn't really matter for the bug report
aoi = pvlib.irradiance.aoi(surface_tilt, surface_azimuth, solar_zenith, solar_azimuth)
pvlib.irradiance.gti_dirint(poa_global, aoi, solar_zenith, solar_azimuth, times, surface_tilt, surface_azimuth)

raises

UnboundLocalError: local variable 'best_ghi' referenced before assignment

The error is not raised e.g. when surface_azimuth = 180.

Versions:

  • pvlib.__version__: 0.9.0
  • pandas.__version__: 1.3.4
  • python: 3.9.4.final.0
@kdebrab
Copy link
Contributor Author

kdebrab commented Nov 24, 2021

The example is not fully correct (I should have converted ghi to the plane before feeding it to gti_dirint), but that doesn't really matter for the bug report.

In this case, the plane doesn't see the sun during the times, so diriving the components would be trivial/impossible (the plane only sees diffuse), but, in any case, the algorithm should not raise an UnboundLocalError.

@cwhanse cwhanse added the bug label Nov 24, 2021
@cwhanse
Copy link
Member

cwhanse commented Nov 24, 2021

Confirmed. The problem appears to be that _gti_dirint_lt_90 is always called, without checking if any aoi value is less than 90. The test for gti_dirint doesn't hit this corner case.

Maybe put an if block around this line.

@wholmgren
Copy link
Member

It's not as simple as putting an if aoi_lt_90.any() around _gti_dirint_lt_90. _gti_dirint_gte_90 requires the best_kt_prime determined in _gti_dirint_lt_90. I don't think we can make this work without going beyond what's in the Marion paper. I suggest we instead return an informative error if insufficient data is supplied with aoi < 90.

@cwhanse cwhanse linked a pull request Dec 31, 2024 that will close this issue
7 tasks
@adriesse
Copy link
Member

adriesse commented Jan 1, 2025

How about just assigning some default values to 'best_ghi' earlier in the code?

@cwhanse
Copy link
Member

cwhanse commented Jan 2, 2025

I'm open to suggestions. best_ghi is the GHI returned from the optimization. Where that can't be done, maybe return np.nan? That seems less helpful to me than interrupting the calculation and informing the user.

@adriesse
Copy link
Member

adriesse commented Jan 5, 2025

np.nan seems ok to me. Either way, the Notes section could explain the problem a bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants