pvlib-python icon indicating copy to clipboard operation
pvlib-python copied to clipboard

Another improvement to pvlib.soiling.hsu documentation

Open ramaroesilva opened this issue 4 months ago • 5 comments

Describe the bug hsu model has one of its parameters depo_veloc, which is described as

Deposition or settling velocity of particulates. [m/s]

The original paper mentions that the equation accepts both deposition and settling velocities (where the 1st comprises various types of particle-fluid interaction, and the 2nd only the gravitational movement of particles). However, the authors found that for their case study the deposition velocity considerably overestimated soiling and that settling velocity performed well - which corresponds to the values assumed in pvlib.

It would be nice if the parameter description could cover this more accurately.

To Reproduce Steps to reproduce the behavior:

  1. Go to hsu documentation
  2. Check printscreens below obtained from paper

Expected behavior Consider as a possible description:

Deposition or settling velocity of particulates. [m/s] The default values correspond to the second case (i.e., only the gravitational movement of particles) since the authors found that the first, which would be rather higher, overestimated soiling for the considered case study.

Screenshots

Image
Image
Image

Versions:

  • pvlib.__version__: 0.13.0

ramaroesilva avatar Aug 18 '25 15:08 ramaroesilva

I generally agree with the recommendation. Re-reading the paper, it is clear that pvlib's default is to use settling velocity in the calculation of mass deposition. The authors considered variable or static deposition velocity, and as @ramaroesilva points out, these are different quantities, and didn't fit the data as well as settling velocity.

  1. I would change the definition of depo_veloc to be "Settling velocity of particulates. [m/s]" It is unfortunate that the parameter was named depo_veloc, which was picked up earlier in the paper, where the term v in Eq. 1 represents any of the various velocities and is describes as "deposition or settling velocity", and it is later that we learn that the authors recommend only settling velocity here.

  2. I support adding a note explaining what the default values of depo_veloc are, but I would do that in a Notes section, not in the parameter description.

cwhanse avatar Aug 21 '25 21:08 cwhanse

We could just call it veloc (still a breaking change!), since the equation itself is flexible, and mention in the Notes section that the default proposed by authors correspond to settling (wishful thinking: would also be nice to include a reference that explains this).

ramaroesilva avatar Aug 22 '25 07:08 ramaroesilva

Hmm. The authors write about this variable as a combination of deposition and settling velocity. I think I retract my comment about changing the definition and parameter name. Would it be sufficient to explain the default value in the Note?

cwhanse avatar Aug 25 '25 15:08 cwhanse

From the paper we can read " v is deposition or settling velocity", so it seems it can be either one. That's why in my previous comment I propose changing to veloc in case if we want to match this flexibility. If we want to avoid a breaking change, we can keep the original name.

Agreed that the main point would be to add a clarification for the default values.

ramaroesilva avatar Aug 25 '25 16:08 ramaroesilva

As your initial comment points out, Eq. 2 shows that they considered $v_d$ as a sum of two terms, a dynamic resistance (which I am inferring is what you mean by deposition velocity) and a settling velocity. So perhaps the authors meant "deposition velocity" more generally than the more specific meaning in your first comment. That's why I'm inclined to leave the definition and name as is.

cwhanse avatar Aug 25 '25 16:08 cwhanse