Skip to content

Fix kimber cleaning threshold and grace period resolution dependence - #2943

Open
CameronGordonn wants to merge 2 commits into
pvlib:mainfrom
CameronGordonn:kimber-cleaning-events
Open

CameronGordonn wants to merge 2 commits into
pvlib:mainfrom
CameronGordonn:kimber-cleaning-events

Conversation

@CameronGordonn

@CameronGordonn CameronGordonn commented Nov 12, 2026 •

Copy link
Copy Markdown
  • Closes kimber grace and rain threshold implementation differences #2796
  • I am familiar with the contributing guidelines
  • I attest that all AI-generated material has been vetted for accuracy and is in compliance with the pvlib license
  • Tests added
  • Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
  • New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
  • Pull request is nearly complete and ready for detailed review.
  • Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

No public API changes, so no updates to docs/sphinx/source/reference.

Fixes the two problems in #2796 with pvlib.soiling.kimber:

  1. Threshold is inclusive. Rain equal to cleaning_threshold now cleans the panels.
  2. Grace period no longer depends on input resolution. A cleaning event now occurs each time cleaning_threshold of new rain has fallen within rain_accum_period, and the grace period lasts grace_period days from that time step.

Why rain is "used up" by an event. Previously, every time step where the rolling 24 h sum exceeded the threshold counted as an event, so the grace period effectively started at the last such step. With finer data there are more such steps, and drizzle after a storm keeps the sum high and pushes the start later. For the hourly Greensboro TMY3 test data, main flags 1950 event hours; this PR flags 251. Counting each threshold's worth of rain once removes that dependence. With daily input and the default 24 h window, it reduces exactly to Kimber (2006)'s rule that a day is a cleaning event if its rainfall meets the threshold (test_kimber_cleaning_events_daily).

Changes in results.

  • greensboro_kimber_soil_nowash.dat and greensboro_kimber_soil_manwash.dat were regenerated by running kimber from this branch on the same TMY3 input and wash date as the existing tests. 634 and 262 of the 8760 hourly values change.
  • With daily input, panels now stay clean for grace_period days after the event day instead of grace_period - 1, consistent with sub-daily input. This is noted in the what's new entry.

Performance. The event search is a loop because each event resets the running sum, but it only visits time steps with rain. On a year of 1-minute data where rain falls in 1–5% of minutes, it takes about 0.04–0.11 s (about 0.03–0.04 s on main). The worst case, with rain every minute, takes about 1.5 s.

🤖 Generated with Claude Code

@github-actions

Copy link
Copy Markdown

Hey @CameronGordonn! 🎉

Thanks for opening your first pull request! We appreciate your
contribution. Please ensure you have reviewed and understood the
contributing guidelines.

If AI is used for any portion of this PR, you must vet the content
for technical accuracy.

Finally, be sure the PR description includes the PR
checklist,
and complete the items you are able to.

Loop over rainy time steps only, since a step without rain cannot
trigger a cleaning event. Note the daily grace period change and link
the PR in the whatsnew entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@CameronGordonn

Copy link
Copy Markdown
Author

Heads up: #2937 (fixing #2935) also changes the timestep code in kimber and removes rain_index_vals, which this PR uses. Whichever merges first, I'll update the other's conflict. On my side, the fix is to pass rainfall.index - rainfall.index[0] to the event helper, which also works for tz-naive data.

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.

kimber grace and rain threshold implementation differences

1 participant