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

Fix trigonometry around sunset and sunrise #10

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

jwohland
Copy link

The current implementation in gsee.trigon has some problems around sunrise and sunset where PV capacities become unreasonably large. This is due to the 1/cos(solar-zenith) term and the way that timestepping is handled.

I suggest a different way to compute the 1/cos contribution that is more representative for cumulative data and can handle non-hourly input data better. The basic idea is to compute 1/cos repeatedly on a sub-timestep scale, set unreasonably large contributions to zero and then average everything. More details can be found here:

background.pdf

I attach plots made using 3h reanalysis data for the initial implementation, using my changes and the difference between both. There are arch-like structures close to the sunset/sunrise lines in individual timesteps (most visible in June and December) using the initial version which vanish in the proposed one.

Initial
pv_3h_CERA20C_year_1901_initial
New
pv_3h_CERA20C_year_1901_new
Difference
pv_3h_CERA20C_year_1901_diff

Open questions:

  • The sub-timestep calculations using N=20 steps slow down the code by a factor of around 5. Maybe one should tweak this a bit or leave it for the user to define in each individual application?
  • In some function call, an additional variable is needed to flag data as either instantaneous (satellite, ground observation) or cumulative (most climate models, reanalysis).

jwohland and others added 4 commits June 17, 2020 15:38
 1 == 2 corresponds to initial implementation and assumes hourly instantaneous radiation input
 2 == 2 calculates the 1/cos(solar_zenith) contribution differently and assumes cumulative radiation data (e.g. from reanalyses or climate models)
- durations need to have a value in current implementation. Set to nan here. Are not used anyway.
@sjpfenninger
Copy link
Member

Thanks @jwohland! I have added a stub to differentiate between accumulated/instantaneous.

I think it would also make sense to test what value works for N and how that affects overall runtime of the code. Also, can you differentiate between the two cases in the unit tests so that those don't fail?

I have also run black on all the main module files. See https://black.readthedocs.io/en/stable/. This auto-formats the code. Can you make sure you run that locally too on making further changes? Hound is not yet set up to account for that, but will be.

jwohland added 4 commits July 14, 2020 10:38
- Updated docstrings to mention that we assume centered data
- output of sun_angles now adapted to specified irradiance type. Output unchange for 'instantaneous'. Only sun_azimuth and timestepmean of (1/cos(zenith) for 'cumulative'.
Copy link

@hound hound bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

./gsee/test/test_trigon.py:120:10: B008: Do not perform function calls in arg...
./gsee/test/test_trigon.py:120:10: B008: Do not perform function calls in argument defaults.  The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call.  If this is intended, assign the function call to a module-level variable and use that variable as a default value.
./gsee/test/test_trigon.py:121:13: B008: Do not perform function calls in argument defaults.  The call is performed only once at function definition time. All calls to your function will reuse the result of that definition-time function call.  If this is intended, assign the function call to a module-level variable and use that variable as a default value.

gsee/trigon.py Show resolved Hide resolved
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 this pull request may close these issues.

2 participants