Skip to content

DOC: clarify default depo_veloc values in soiling.hsu Notes - #2854

Open
weixuan-song wants to merge 2 commits into
pvlib:mainfrom
weixuan-song:doc-hsu-depo-veloc-note
Open

DOC: clarify default depo_veloc values in soiling.hsu Notes#2854
weixuan-song wants to merge 2 commits into
pvlib:mainfrom
weixuan-song:doc-hsu-depo-veloc-note

Conversation

@weixuan-song

@weixuan-song weixuan-song commented Sep 5, 2026

Copy link
Copy Markdown

Description

Addresses #2535.

As discussed in the issue, the depo_veloc parameter of soiling.hsu is often left at its default (None), and the existing documentation did not make clear what those default values mean. The original paper notes that the velocity term in the model can represent either a deposition velocity (which accounts for various particle–fluid interactions beyond gravity) or a settling velocity (the gravitational movement of particles only). The authors found that using the deposition velocity considerably overestimates soiling, while the settling velocity performed well in their case study — and pvlib's default depo_veloc values correspond to the settling-velocity case.

This PR expands the Notes section of soiling.hsu to make that distinction explicit: the default depo_veloc values correspond to the settling velocity (gravitational motion only), and the model also accepts deposition velocities, which comprise other particle–fluid interactions and typically overestimate soiling.

Changes:

  • pvlib/soiling.py: expanded the Notes section of soiling.hsu clarifying the default depo_veloc values and the meaning of settling velocity (default) vs. deposition velocity.
  • docs/sphinx/source/whatsnew/v0.16.0.rst: added a Documentation entry referencing :issue:2535.

No code behavior is changed; this is a documentation-only change.

Checklist

  • Pull request is written in English
  • All tests pass locally (the hsu-related tests in pvlib/tests/test_soiling.py pass)
  • Added whatsnew entry

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

Hey @weixuan-song! 🎉

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.

@williamhobbs

Copy link
Copy Markdown
Contributor

@weixuan-song, what are machine_mass and tilt_and_capsule?

@weixuan-song

Copy link
Copy Markdown
Author

Thanks for the question, @williamhobbs. Those names (machine_mass, tilt_and_capsule, capsule) were a mistake in the PR description — they are not actual parameters or modes anywhere in the code.

To clarify: the depo_veloc parameter of soiling.hsu is a dict keyed by particle-size cutoff (default {"2_5": 0.0009, "10": 0.004}), not a mode selector. My intention was only to explain what the default values mean, not to introduce any mode names. The distinction described in the Notes is between two physical quantities:

  • settling velocity — only the gravitational movement of particles (which is what pvlib's defaults correspond to)
  • deposition velocity — a broader quantity that includes other particle-fluid interactions, which the paper's authors found considerably overestimates soiling

I have updated the PR description to remove those erroneous mode names and describe the change accurately. The code change itself (the expanded Notes in soiling.hsu) only discusses the settling vs. deposition velocity distinction, consistent with issue #2535. Please let me know if anything else is unclear.

Comment thread pvlib/soiling.py Outdated
Comment on lines +70 to +76
The default values of ``depo_veloc`` correspond to the settling
velocity of particulates, i.e., only the gravitational movement of
particles. The model also accepts deposition velocities, which
comprise other types of particle-fluid interaction; however, the
authors of [1]_ found that deposition velocity considerably
overestimates soiling, while settling velocity performed well in
their case study.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
The default values of ``depo_veloc`` correspond to the settling
velocity of particulates, i.e., only the gravitational movement of
particles. The model also accepts deposition velocities, which
comprise other types of particle-fluid interaction; however, the
authors of [1]_ found that deposition velocity considerably
overestimates soiling, while settling velocity performed well in
their case study.
As recommended by [1]_, the default values of ``depo_veloc``
are the settling velocities of particulates, i.e., only the gravitational
movement of particles.

@weixuan-song

Copy link
Copy Markdown
Author

Thanks for the suggestion, @cwhanse. I have applied it exactly as proposed — the Notes section now reads "As recommended by [1]_, the default values of depo_veloc are the settling velocities of particulates, i.e., only the gravitational movement of particles.", with the extra sentences removed. Pushed to the PR branch as commit a2120e0. This is a documentation-only change; no code behavior is altered.

1 similar comment
@weixuan-song

Copy link
Copy Markdown
Author

Thanks for the suggestion, @cwhanse. I have applied it exactly as proposed — the Notes section now reads "As recommended by [1]_, the default values of depo_veloc are the settling velocities of particulates, i.e., only the gravitational movement of particles.", with the extra sentences removed. Pushed to the PR branch as commit a2120e0. This is a documentation-only change; no code behavior is altered.

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.

3 participants