Open kandersolar opened 2 years ago
hi @kanderso-nrel , sorry for the delay on this. And thanks a lot for posting this issue and for the super helpful notebook 🙏 This is clearly a bug. @campanelli-sunpower could you mark it as such please?
I spent a couple of hours digging and I found an "easy" way to circumvent the bug and avoid the discontinuity using the inputs, but I couldn't find a way to fix the bug because of lack of time.
I could reproduce the issue with a notebook also here: (0: flat, 1: tilted west, 2: tilted east)
The problem arises from the value of surface_azimuth
when surface_tilt == 0
. In the above, when we use surface_azimuth = 90
for the flat situation we have the discontinuity. But when using surface_tilt = 270
when flat, the discontinuity disappears:
I think the bug is related to some ground surface indexing issue when surface_tilt=0
which messes up the view factor calculation as you pointed out; I think that fixing it is a matter of understanding why the view factor calculator doesn't understand the surface indexes correctly, but I would need more time to keep searching.
Ah ha, changing surface_azimuth
to 270 solves the problem for me as well! Good find @anomam. In fact it seems that any surface_azimuth
from 180 to 360 works, while azimuths in (0, 180)
show the discontinuity.
Some uninformed speculation: I wonder if that ground surface indexing difference could somehow be related to the NaN ground cutting points? I will try to find some time to look into it myself, but I can't promise that will happen any time soon.
@kanderso-nrel - thanks for bringing this issue up. And @anomam - thank you for the reply. Are you aware of any link in this issue between axis_azimuth
and surface_azimuth
. I was able to bypass this issue with some patchy work of manipulating the axis_azimuth
as a function of surface_azimuth
. Admittedly I am using the pvlib
wrapper on this testing.
I found that adjusting axis_azimuth
corrects the problem in the same or near-same way. Essentially, if surface_azimuth
is 90, setting axis_azimuth
to 0 corrects (and if surface_azimuth
is 270, setting axis_azimuth
to 180 corrects).
Indeed there is a dependence on axis_azimuth
. Here is a plot of the axis_azimuth x surface_azimuth
plane (discretized in 10-degree steps, hence the "chunkiness"), with yellow indicating incorrect results. So "stay in the purple" for consistent results.
As always I will preface this by saying I'm not a frequent pvfactors user or even someone that has read the pvfactors PVSC paper, so there's a chance this is user error. @spaneja showed this behavior to me and I tried to identify the root cause before opening the issue.
The high-level issue is essentially that the simulated irradiance values for
surface_tilt=0
andsurface_tilt=0.001
are very different, despite no practical difference insurface_tilt
. Here is an example output where the only difference is whethersurface_tilt
is 0 or 0.001:I have uploaded a small notebook to reproduce the issue here: https://gist.github.com/kanderso-nrel/4c780640585881b01f82768c5bac5b51
I think I found two issues in the pvfactors code caused by tilt being exactly zero but it's possible that there are more. The first is pretty simple, and mentioned as a warning in that notebook (
divide by zero encountered in true_divide
). The problem is that finding the cutting points inpvground.py
fails when the modules are horizontal. This makes theoretical sense (two parallel lines never intersect), but I'm not sure what to do about it in the code.https://github.com/SunPower/pvfactors/blob/acfb7e2273667ed4a75890b3ac45fc741af9b3d5/pvfactors/geometry/pvground.py#L121
The second is that when
surface_tilt=0
, it seems that some values inpvarray.ts_vf_aoi_matrix
go negative, which I'm assuming is unexpected. However the view factor calculation code is more than I want to chew on right now so I did not determine what code is to blame for it.I'm happy to help investigate further but I suspect it would be very inefficient given my lack of familiarity with view factor geometry, so I'm hoping someone more experienced can do it instead 😁