-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dynamic Snow Loss Model #2044
Comments
Now that's what I call well commented code! The functional programming is perfectly fine. OOP can be tackled down in another PR. Just a heads up, that's a pretty big model, so be patient to get it all working and reviewed. In any case, that's gonna be a nice contribution! Recommended readings and tips:
|
Hey Luis,
Thank you for the tips! Once I functionalize the code and implement your tips should I reply to the issue with the new attached code?
Thanks,
Kyle
…________________________________
From: Echedey Luis ***@***.***>
Sent: Friday, May 10, 2024 3:40 PM
To: pvlib/pvlib-python ***@***.***>
Cc: kylefmacdonald ***@***.***>; Author ***@***.***>
Subject: Re: [pvlib/pvlib-python] Dynamic Snow Loss Model (Issue #2044)
Now that's what I call well commented code! The functional programming is perfectly fine. OOP can be tackled down in another PR.
Just a heads up, that's a pretty big model, so be patient to get it all working and reviewed. In any case, that's gonna be a nice contribution!
Recommended readings and tips:
* https://pvlib-python.readthedocs.io/en/stable/contributing.html
* rST cheatsheet, to redact documentation https://bashtage.github.io/sphinx-material/rst-cheatsheet/rst-cheatsheet.html
* Develop in the lowest python version supported (now it is 3.8), so things don't fail unexpectedly
* Install the package in editable mode at least with the test optional packages in a virtual environment
python -m venv venv
venv/Scripts/activate or venv/bin/activate
python -m install -e .[test]
—
Reply to this email directly, view it on GitHub<#2044 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL62C4QCAHPLZIFJXX55I63ZBU5DPAVCNFSM6AAAAABHRHJU3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBVGMYDEOBUHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Feel free to do what you need, although I'd leave this open at least as a possible addition. In any case, if you go further with the code, opening a pull request is possibly a better fit! (Btw, I'm just a contributor, I wouldn't be able to close your issue) |
It would be too boring if I told you everything that must be done. It's a requirement to figure out some of the changes ;). What's more, isn't open source just another form of human creativity? I'd say it's the finest one. Seriously, lurk over the repo, it will also help you be confident with it. |
Kyle,
I'm confused - why would you like the issue deleted? Normally, we leave issues open until corresponding pull requests are merged. Or the originator (you) should be able to close the issue, if you want, but I don't see the need. Adding that snow loss model improvement seems worthwhile to me.
Cliff
From: kylefmacdonald ***@***.***>
Sent: Monday, May 13, 2024 4:20 PM
To: pvlib/pvlib-python ***@***.***>
Cc: Subscribed ***@***.***>
Subject: [EXTERNAL] Re: [pvlib/pvlib-python] Dynamic Snow Loss Model (Issue #2044)
Hey Luis,
I would like to take this issue down for now. Could you please forward this request to a maintainer so this issue can be deleted. Once I get permission, I will create a pull request as you suggested. However, until then I would like to delete this issue.
Thank you,
Kyle
From: Echedey Luis ***@***.***<mailto:***@***.***>>
Date: Friday, May 10, 2024 at 9:18 PM
To: pvlib/pvlib-python ***@***.***<mailto:***@***.***>>
Cc: kylefmacdonald ***@***.***<mailto:***@***.***>>, State change ***@***.***<mailto:***@***.***>>
Subject: Re: [pvlib/pvlib-python] Dynamic Snow Loss Model (Issue #2044)
It would be too boring if I told you everything that must be done. It's a requirement to figure out some of the changes ;). What's more, isn't open source just another form of human creativity? I'd say it's the finest one.
Seriously, lurk over the repo, it will also help you be confident with it.
-
Reply to this email directly, view it on GitHub<#2044 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL62C4VOLT7AEM5RSVHSAZDZBVWWFAVCNFSM6AAAAABHRHJU3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBVGQZTANJXGU>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***<mailto:***@***.***>>
-
Reply to this email directly, view it on GitHub<#2044 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABJE2L64ZYTWKSSQTB4YNDLZCE37HAVCNFSM6AAAAABHRHJU3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBYHA4TSMRSGA>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.******@***.***>>
|
Hey Chris,
Thank you for following up. I agree, I do think it would be a notable project. However I am waiting for approval from my company to see if I can work on this outside of work before I proceed which is why I wanted to delete the issue. I misunderstood what creating an issue meant. However I will leave it up while I wait for an answer from my company.
Thank you,
Kyle
…________________________________
From: Cliff Hansen ***@***.***>
Sent: Tuesday, May 14, 2024 10:30 AM
To: pvlib/pvlib-python ***@***.***>
Cc: kylefmacdonald ***@***.***>; State change ***@***.***>
Subject: Re: [pvlib/pvlib-python] Dynamic Snow Loss Model (Issue #2044)
Kyle,
I'm confused - why would you like the issue deleted? Normally, we leave issues open until corresponding pull requests are merged. Or the originator (you) should be able to close the issue, if you want, but I don't see the need. Adding that snow loss model improvement seems worthwhile to me.
Cliff
From: kylefmacdonald ***@***.***>
Sent: Monday, May 13, 2024 4:20 PM
To: pvlib/pvlib-python ***@***.***>
Cc: Subscribed ***@***.***>
Subject: [EXTERNAL] Re: [pvlib/pvlib-python] Dynamic Snow Loss Model (Issue #2044)
Hey Luis,
I would like to take this issue down for now. Could you please forward this request to a maintainer so this issue can be deleted. Once I get permission, I will create a pull request as you suggested. However, until then I would like to delete this issue.
Thank you,
Kyle
From: Echedey Luis ***@***.***<mailto:***@***.***>>
Date: Friday, May 10, 2024 at 9:18 PM
To: pvlib/pvlib-python ***@***.***<mailto:***@***.***>>
Cc: kylefmacdonald ***@***.***<mailto:***@***.***>>, State change ***@***.***<mailto:***@***.***>>
Subject: Re: [pvlib/pvlib-python] Dynamic Snow Loss Model (Issue #2044)
It would be too boring if I told you everything that must be done. It's a requirement to figure out some of the changes ;). What's more, isn't open source just another form of human creativity? I'd say it's the finest one.
Seriously, lurk over the repo, it will also help you be confident with it.
-
Reply to this email directly, view it on GitHub<#2044 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL62C4VOLT7AEM5RSVHSAZDZBVWWFAVCNFSM6AAAAABHRHJU3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBVGQZTANJXGU>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***<mailto:***@***.***>>
-
Reply to this email directly, view it on GitHub<#2044 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABJE2L64ZYTWKSSQTB4YNDLZCE37HAVCNFSM6AAAAABHRHJU3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMBYHA4TSMRSGA>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.******@***.***>>
—
Reply to this email directly, view it on GitHub<#2044 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL62C4SCAWUJ6TKDOMAWCPTZCINXRAVCNFSM6AAAAABHRHJU3WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMJQGQYDAMRSG4>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
I understand. Just to be clear, opening the issue doesn't obligate you to provide code. |
Is your feature request related to a problem? Please describe.
Snow loss modelling is difficult and currently the only model available on pvlib at an hourly timestep is the Marion model:
https://pvlib-python.readthedocs.io/en/v0.10.4/reference/generated/pvlib.snow.coverage_nrel.html
However this model does not account for the movement of a tracker and the snow pile that may accumulate.
Describe the solution you'd like
In 2018, Defne Gun, Mike Anderson, Greg Kimball, and Ben Bourne developed a dynamic snow loss model:
https://gregorykimball.me/wp-content/uploads/2018/07/mewcpec1079_0614162840_snow_2018.pdf
My solution would be to contribute this code to pvlib so the industry may model snow losses accounted for the dynamic nature of Horizontal Single Axis Tracker (HSAT) systems. I wrote code 2 years ago when I was relatively new to python. I would appreciate any feedback and advice you have however in the meantime I will also review my code as I am sure it can be improved. After an initial look I will start by refactoring to functional programming (I can also refactor to OOP if that is what you would prefer). In addition I will also update the code to follow the pvlib guidelines.
Describe alternatives you've considered
No other dynamic model currently exists in pvlib, I am simply using a model that has already been developed and released publicly.
Thank you in advance for the guidance and help on this, looking forward to contributing!
The text was updated successfully, but these errors were encountered: