pvlib.soiling.hsu model implementation errors #970
Comments
|
nice sleuthing Josh! Is a PR forthcoming? 🎉 |
|
Hi Mark,
Yes, a PR is in the works. I need to improve the testing first.
…-Josh
From: Mark Mikofski <[email protected]>
Reply-To: pvlib/pvlib-python <[email protected]>
Date: Tuesday, May 19, 2020 at 3:51 PM
To: pvlib/pvlib-python <[email protected]>
Cc: Joshua Stein <[email protected]>, Author <[email protected]>
Subject: [EXTERNAL] Re: [pvlib/pvlib-python] pvlib.soiling.hsu model implementation errors (#970)
nice sleuthing Josh! Is a PR forthcoming? 🎉
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#970 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABJES5C2CRTZFF7ROT2EPOTRSL5ORANCNFSM4NFL4K3Q>.
|
|
Now I need to go back and figure out where I missed these errors in the review. |
Nah, IMHO this right here is why I ❤️ open science. Everyone makes a mistake, even the best, and it may go unnoticed perhaps for a long time, but when used in earnest by many, important issues are caught & fixed more quickly. This right here is the proof! |
jsstein
added a commit
to jsstein/pvlib-python
that referenced
this issue
Jun 9, 2020
jsstein
added a commit
to jsstein/pvlib-python
that referenced
this issue
Jun 9, 2020
7 tasks
jsstein
added a commit
to jsstein/pvlib-python
that referenced
this issue
Jun 9, 2020
jsstein
added a commit
to jsstein/pvlib-python
that referenced
this issue
Jun 9, 2020
cwhanse
pushed a commit
that referenced
this issue
Jun 11, 2020
|
Some improvements are needed in #977 |
6 tasks
wholmgren
pushed a commit
that referenced
this issue
Aug 28, 2020
…r than 1 hour) (#980) * bug fixes to hsu soiling function. Addresses (#970) * Made changes to the soiling tests Addresses (#970) * update what's new documentation * Fixed stickler-ci issues in test_soiling.py * Fixed errant character in test * allowed function to accept aribtrary time intervals * updated v0.8.0.rst file * Add variable time interval functionality and testing to soiling.hsu(), also corrected horiz_mass_rate equation * fixed typo in test_soiling.py * fixed stickler-ci errors * fixed more formatting errors * more stickler-ci issues fixed * fixed more stickler-ci issues * formatting issues * formatting issues * formatting... * formatting..... * changed variable time interval calculation in hsu model and test * formatting changes * formatting * formatting * formatting * whatsnew PR links * remove needs_pandas_22 from test_soiling * move time interval note from bug fixes to enhancements * stickler * review improvements Co-authored-by: Kevin Anderson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
I ran an example run using the Matlab version of the HSU soiling function and found that the python version did not give anywhere near the same results. The Matlab results matched the results in the original JPV paper. As a result of this test, I found two errors in the python implementation, which are listed below:
depo_veloc = {'2_5': 0.004, '10': 0.0009} has the wrong default values. They are reversed.
The proper dictionary should be: {'2_5': 0.0009, '10': 0.004}. This is confirmed in the JPV paper and the Matlab version of the function.
The horiz_mass_rate is in g/(m^2hr) but should be in g/(m^2s). The line needs to be multiplied by 60x60 or 3600.
The proper line of code should be:
horiz_mass_rate = (pm2_5 * depo_veloc['2_5']+ np.maximum(pm10 - pm2_5, 0.) * depo_veloc['10'])*3600
When I made these changes I was able to match the validation dataset from the JPV paper, as shown below.
The text was updated successfully, but these errors were encountered: