Another improvement to pvlib.soiling.hsu documentation
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:
- Go to
hsudocumentation - 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
Versions:
pvlib.__version__: 0.13.0
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.
-
I would change the definition of
depo_velocto be "Settling velocity of particulates. [m/s]" It is unfortunate that the parameter was nameddepo_veloc, which was picked up earlier in the paper, where the termvin 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. -
I support adding a note explaining what the default values of
depo_velocare, but I would do that in a Notes section, not in the parameter description.
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).
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?
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.
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.