You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
kimber grace and rain threshold implementation differences #2796
Thanks for reporting this — I agree the current > check is too strict compared with Kimber 2006. I’ll update the Kimber implementation so rainfall equal to cleaning_threshold also counts as a cleaning event, and I’ll fix the grace-period indexing so grace_period=1 means one full day after rainfall rather than including the rain day itself. I’ll add tests for both cases and submit a PR.
set grace days to 1, and effectively here are no grace days!
I think this only happens if the input rainfall data are 1-day timesteps. The test data are hourly, the proposed fix "closed='both'" is just shifting the grace period by one time step.
Edit: revised the proposed rule and script shortly after posting; the first version had a bug where multi-day rain could trigger cleaning.
@dgowdaan-cmyk#2814 has been quiet since July. Unless you're still working on it, I'd like to open a new PR with the approach below. @kandersolar does that work?
Kimber et al. (2006) backs up both parts of the issue (p. 2394):
Threshold: "the amount of rain required to fall in one day to fully clean the PV system", so >=.
Grace period: "the number of days that a system remains relatively clean after the last rain that meets the cleaning threshold."
The grace period also depends on data resolution. Suggested fix: each time a threshold's worth of new rain falls within rain_accum_period, that's a cleaning event and the grace period starts then. With the default 24 h, that's exactly Kimber's daily rule, and resolutions agree to within one timestep (daily totals are stamped at end of day):
10 mm, Jan 4 12:00-15:00, grace_period=1, clean until:
current proposed
15 min Jan 06 12:45 Jan 05 14:00
1 h Jan 06 12:00 Jan 05 14:00
1 day Jan 05 00:00 Jan 06 00:00
Trade-offs: grace counts from when the threshold is reached, not the last rain. A 12 mm storm counts twice and 11.9 mm once, so grace starts hours later. It's a loop (~1.4 s per year of 1-min data). NaN rain counts as zero, same as now.
Alternatives: counting continuous rain after the threshold is vectorized and starts grace at the end of the storm, but on daily data a week of drizzle days restarts the grace period every day. Just >= plus closed='both' is the smallest change but keeps the resolution dependence.
@mikofski I'd call this a bug fix, not a deprecation, since the signature doesn't change. Greensboro annual soiling loss: hourly 0.100% -> 0.111%, daily 0.108% -> 0.094% (hourly currently gets too much grace, daily too little).
Which approach do you prefer?
AI helped with the script, the wording, and comparing fix designs. I chose the approach and checked the paper and the results.
script
"""pvlib #2796: kimber's grace period depends on input resolution."""importnumpyasnpimportpandasaspdfrompvlib.soilingimportkimberdefcleaning_events(rain, threshold=6, window='24h', tol=1e-6):
"""An event each time `threshold` of new rain falls within `window`. The event uses up that rain, so later drizzle can't trigger another."""t=rain.index.to_numpy()
r=np.nan_to_num(rain.to_numpy(dtype=float)) # NaN counts as no rainwin=pd.Timedelta(window).to_timedelta64()
events=np.zeros(len(r), dtype=bool)
start, total=0, 0.0foriinrange(len(r)):
total+=r[i]
whilet[i] -t[start] >=win:
total-=r[start]
start+=1iftotal>=threshold-tol: # tolerance for float errorevents[i] =Truestart, total=i+1, 0.0returnpd.Series(events, rain.index)
defclean_until(clean):
"""Last timestamp of the first clean stretch (skipping t=0)."""after=clean.iloc[1:][clean.iloc[1:].idxmax():]
returnafter.index[after.argmin() -1]
# 10 mm of rain from 12:00 to 15:00 on Jan 4, recorded at 15-min resolutionidx=pd.date_range('2020-01-01', '2020-01-09', freq='15min', tz='UTC')
rain=pd.Series(0.0, idx)
rain['2020-01-04 12:15':'2020-01-04 15:00'] =10/12print('grace_period=1, clean until: current proposed')
forfreqin ['15min', '1h', '1D']:
r=rain.resample(freq, label='right', closed='right').sum()
current=kimber(r, grace_period=1) ==0proposed=cleaning_events(r).rolling('1D', closed='both').sum() >0c, p= (f'{clean_until(x):%b %d %H:%M}'forxin (current, proposed))
print(f'{freq:>6}{c:>21}{p:>15}')
# rain exactly at the threshold never cleansr=pd.Series([0, 0, 6.0, 0],
pd.date_range('2020-01-01', periods=4, freq='D', tz='UTC'))
print('6.0 mm, threshold 6:', kimber(r).round(4).tolist())
grace_period=1, clean until: current proposed
15min Jan 06 12:45 Jan 05 14:00
1h Jan 06 12:00 Jan 05 14:00
1D Jan 05 00:00 Jan 06 00:00
6.0 mm, threshold 6: [0.0, 0.0015, 0.003, 0.0045]
Activity
dgowdaan-cmyk commentedon Jul 2, 2026
Thanks for reporting this — I agree the current > check is too strict compared with Kimber 2006. I’ll update the Kimber implementation so rainfall equal to cleaning_threshold also counts as a cleaning event, and I’ll fix the grace-period indexing so grace_period=1 means one full day after rainfall rather than including the rain day itself. I’ll add tests for both cases and submit a PR.
cwhanse commentedon Jul 11, 2026
I think this only happens if the input rainfall data are 1-day timesteps. The test data are hourly, the proposed fix "closed='both'" is just shifting the grace period by one time step.
CameronGordonn commentedon Oct 30, 2026
Edit: revised the proposed rule and script shortly after posting; the first version had a bug where multi-day rain could trigger cleaning.
@dgowdaan-cmyk #2814 has been quiet since July. Unless you're still working on it, I'd like to open a new PR with the approach below. @kandersolar does that work?
Kimber et al. (2006) backs up both parts of the issue (p. 2394):
>=.The grace period also depends on data resolution. Suggested fix: each time a threshold's worth of new rain falls within
rain_accum_period, that's a cleaning event and the grace period starts then. With the default 24 h, that's exactly Kimber's daily rule, and resolutions agree to within one timestep (daily totals are stamped at end of day):Trade-offs: grace counts from when the threshold is reached, not the last rain. A 12 mm storm counts twice and 11.9 mm once, so grace starts hours later. It's a loop (~1.4 s per year of 1-min data). NaN rain counts as zero, same as now.
Alternatives: counting continuous rain after the threshold is vectorized and starts grace at the end of the storm, but on daily data a week of drizzle days restarts the grace period every day. Just
>=plusclosed='both'is the smallest change but keeps the resolution dependence.@mikofski I'd call this a bug fix, not a deprecation, since the signature doesn't change. Greensboro annual soiling loss: hourly 0.100% -> 0.111%, daily 0.108% -> 0.094% (hourly currently gets too much grace, daily too little).
Which approach do you prefer?
AI helped with the script, the wording, and comparing fix designs. I chose the approach and checked the paper and the results.
script
cwhanse commentedon Nov 9, 2026
@CameronGordonn please go ahead with a PR, if you are still willing. Please make sure to include the checklist. Thanks!
CameronGordonn commentedon Nov 12, 2026
Thanks @cwhanse, PR is up at #2943. It also includes the checklist.