-
Notifications
You must be signed in to change notification settings - Fork 7
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
Kempe standoff pull request #58
Conversation
overall major reworking of the standoff calculations. Too many things to list.
Just added in a print statement to indicate the humidity was finished calculating.
Just a bunch of updates to make it into the tool I want.
I updated it to have a theoretical measured set of module data.
Just small formating issues here.
Major revision underway. I have most of the program working but the last few sections are still needing a bit of work.
Lots of reworking on the code to get it to have more methods for calculations. there is still a lot to be done.
Put in defaults in get_NSRDB
just working on it
Put in code to start a get_satellite function
typos fixed
fixed now.
Hopefully now it is patched correctly.
now it could work.
fixed import nsrdbx as f
updated
Updated the demo module data to be calculated from the cell temperature instead of the module surface temperature.
Updated the manufactured module data.
Incldues the modeled POA data from Python and an updated module temperature calculation.
I changed it to instead of just having 180 degrees south be the default azimuth, it will point north or 0 degrees by default in the southern hemisphere.
It's a complete overhaul. I'm going to merge it now, but I would like it put in the main as is and to have it's name changed to "Standoff.ipynb"
Making the default azimuth be equatorial facing instead of always south. No northern-centric bias here. ;)
I fixed the cell temperature calculation to default to a wind speed factor of 1.7 instead of 1 and to allow that to be passed in. My guess is that this change will cause problems elsewhere.
Massive changes here but for the most part it is all about the standoff calculation which is working. One change to be aware of is that the prior calculations were using the module surface temperature for the standoff calculation but now it is using the cell temperature calculation.
It now uses the desired power factor for wind speed height adjustment.
It now uses the desired power factor for wind speed height adjustment. I found an error in the standoff calculation. It was previously using the module surface temperature and it should be using the module cell temperature. It makes the required standoff calculations a bit higher.
Fixed some documentation for wind speed dependence and made it so the T98 calculation will default to equatorial facing, open rack and latitude tilt.
fixed the improper importing NSRDBX as f and did some documentation work.
Fixed the functions with capitals issue.
Fixed the functions with capitals issue.
I fixed an error check for the case of now wind height specification in the meta data of a dataset by removing it. that error check is managed in temperature.py.
Just some documentation changes.
just some documentation changes
@MDKempe - This PR is now ready to be merged into development. -) I excluded the changes in utitlities.py - You can still find those in a new branch called Kempe_standoff_dev All tests are now passing with all the different python versions. Please review my changes and let me know if you'd like certain revisions differently. Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few unsolicited comments from the peanut gallery regarding the temperature models
elif temp_model == "ross": | ||
wind_speed_factor = ( | ||
10 / float(meta["wind_height"]) | ||
) ** wind_factor # I had to guess what this one was |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Ross model does not consider wind speed, so it has no adjustment height
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I'd suggest we delete that option but leave in a note so someone looking at the code won't waste their time wondering why that one was left out.
wind_speed_factor = ( | ||
1 # The wind speed height is managed weirdly for this one. | ||
) | ||
elif temp_model == "prilliman": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that the prilliman model is different from the others; rather that modeling a steady-state cell temperature as SAPM, faiman, etc do, it converts a modeled steady-state cell temperature into a transient cell temperature. So it's a post-processing step on the other models. I don't know the context of this code, I just thought it was unusual to see prilliman listed as an alternative to the other models, since conceptually it is not the same species of model as the others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. At this point I just put in placeholders for the different temperature models as listed in pvlib. So It sounds like we should just delete that as a future option. So is calling it directly from pvlib the appropriate thing to do? or does it make sense to write code in PVdeg to make its use easier? Would code in pvdeg be a stand alone method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with PVdeg at all, I just happened to look at this PR :) So I can't comment on what is suitable for PVdeg to do.
One tidbit I can offer is that the Prilliman model only takes effect in simulations with time step < 20 minutes, which might make it wholly irrelevant if the focus here is on hourly data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't know what people will want to do at this time. But leaving it open for people to work on and implement later is reasonable. Long term someone will want to do an interpolation of the data. I'd have to look into this model to see if it is something I would want to use. None of these models really capture the transients properly anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not constrained to hourly data, but we can add a check so it's only an option for sub-hourly data of 20 or less intervals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presently, it is just a place holder. How about we just leave it in and whoever works on it to get this model working can determine what is best.
Thank you @kandersolar. Every input is very much appreciated! |
Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com>
Welcome to Codecov 🎉Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment. Thanks for integrating Codecov - We've got you covered ☂️ |
* add pypi workflow * Update publish-to-pypi.yml * update development (#59) * Reworking of the standoff calculations * standard updates * Update weather.py Just added in a print statement to indicate the humidity was finished calculating. * updates to the standoff calculation Just a bunch of updates to make it into the tool I want. * Update xeff_demo.csv I updated it to have a theoretical measured set of module data. * Update standards.py Just small formating issues here. * Create xeff_demo.xlsx * Adding calculations to the file Major revision underway. I have most of the program working but the last few sections are still needing a bit of work. * Update 4 - Standards.py Lots of reworking on the code to get it to have more methods for calculations. there is still a lot to be done. * Update weather.py Put in defaults in get_NSRDB * Update standards.py just working on it * Update weather.py Put in code to start a get_satellite function * Update weather.py typos fixed * Update weather.py fixed now. * Update weather.py Hopefully now it is patched correctly. * Update weather.py now it could work. * Update weather.py * Update weather.py fixed import nsrdbx as f * Update weather.py updated * Update 4 - Standards.ipynb update * standards * Update xeff_demo.csv Updated the demo module data to be calculated from the cell temperature instead of the module surface temperature. * Update xeff_demo.xlsx * Update xeff_demo.csv Updated the manufactured module data. * Update xeff_demo.xlsx Incldues the modeled POA data from Python and an updated module temperature calculation. * Update utilities.py I changed it to instead of just having 180 degrees south be the default azimuth, it will point north or 0 degrees by default in the southern hemisphere. * Massive changes and a stopping point It's a complete overhaul. I'm going to merge it now, but I would like it put in the main as is and to have it's name changed to "Standoff.ipynb" * Update spectral.py Making the default azimuth be equatorial facing instead of always south. No northern-centric bias here. ;) * Update temperature.py I fixed the cell temperature calculation to default to a wind speed factor of 1.7 instead of 1 and to allow that to be passed in. My guess is that this change will cause problems elsewhere. * Update standards.py Massive changes here but for the most part it is all about the standoff calculation which is working. One change to be aware of is that the prior calculations were using the module surface temperature for the standoff calculation but now it is using the cell temperature calculation. * improvement on temperature windspeedfactor fix * dding pytest coverage instructions * Wind height added Added in a field for wind height data to the demonstration weather data. * Update weather.py Added in some if/then statements to put in a field for the wind speed measurement height. For the NSRDB it is set for 2m, for PVGIS it is set for 10m. * Update weather.py Made it so it would delete the message indicating that Rh was being calculated once it is completed. * Update standards.py Created a print statement method to output the results of the standoff calculation. * Update temperature.py Changed it to do a wind speed factor calculation based on the wind height in the meta data and the supplied power factor using appropriate defaults. * Update temperature.py Got it to correctly use the wind speed exponent. * Update standards.py Got it to correctly use the wind speed exponent. * Update 4 - Standards.ipynb Got it to correctly use the wind speed exponent. * Update temperature.py Fixing wind speed stuff * Update 4 - Standards.ipynb Fixing wind speed stuff * Update fatigue.py Modified the fatigue calculator to work with the new wind speed exponent method. * Update test_fatigue.py Fixed this procedure to call the module temperature function correctly. * Update humidity.py It now uses the desired power factor for wind speed height adjustment. * Update h5_pytest.h5 It now uses the desired power factor for wind speed height adjustment. * Update meta.json It now has a field for the wind speed height. * Update test_humidity.py It now uses the desired power factor for wind speed height adjustment. * Update test_standards.py It now uses the desired power factor for wind speed height adjustment. * Update test_temperature.py It now uses the desired power factor for wind speed height adjustment. * Update standards.py It now uses the desired power factor for wind speed height adjustment. I found an error in the standoff calculation. It was previously using the module surface temperature and it should be using the module cell temperature. It makes the required standoff calculations a bit higher. * Update standards.py Fixed some documentation for wind speed dependence and made it so the T98 calculation will default to equatorial facing, open rack and latitude tilt. * Update weather.py fixed the improper importing NSRDBX as f and did some documentation work. * Update standards.py Fixed the functions with capitals issue. * Update 4 - Standards.ipynb Fixed the functions with capitals issue. * deploy to pypi on tag release (#55) * add pypi workflow * Update standards.py I fixed an error check for the case of now wind height specification in the meta data of a dataset by removing it. that error check is managed in temperature.py. * Update 4 - Standards.ipynb Just some documentation changes. * Update temperature.py just some documentation changes * run pre-commit hooks * revert changes in h5_pytest.h5 * update docs conf from main * Revert "Kempe standoff" (#57) * run updated pre-commit hooks * add pre-commit to dependencies --------- Co-authored-by: MDKempe <58960264+MDKempe@users.noreply.github.com> Co-authored-by: Silvana Ovaitt <silvana.ayala@nrel.gov> Co-authored-by: Kempe <mkempe@nrel.gov> Co-authored-by: Silvana Ovaitt <Silvana.Ovaitt@nrel.gov> * added codecov to actions * write coverage report * update pytest.yml * add pytest-cov * add build status to readme * Kempe standoff pull request (#58) * Reworking of the standoff calculations * standard updates * Update weather.py * Update xeff_demo.csv * Create xeff_demo.xlsx * Put in defaults in get_NSRDB * typos fixed * Changed standoff calculation to instead of just having 180 degrees south be the default azimuth, it will point north or 0 degrees by default in the southern hemisphere. * Update spectral.py * Massive changes here but for the most part it is all about the standoff calculation which is working. One change to be aware of is that the prior calculations were using the module surface temperature for the standoff calculation but now it is using the cell temperature calculation. * improvement on temperature windspeedfactor fix * adding pytest coverage instructions * Wind height added * Added in a field for wind height data to the demonstration weather data. * Added in some if/then statements to put in a field for the wind speed measurement height. For the NSRDB it is set for 2m, for PVGIS it is set for 10m. * Made it so it would delete the message indicating that Rh was being calculated once it is completed. * Changed it to do a wind speed factor calculation based on the wind height in the meta data and the supplied power factor using appropriate defaults. * Modified the fatigue calculator to work with the new wind speed exponent method. * Update meta.json * Update test_humidity.py * Update test_standards.py * Update test_temperature.py * Update standards.py * I found an error in the standoff calculation. It was previously using the module surface temperature and it should be using the module cell temperature. It makes the required standoff calculations a bit higher. * Update standards.py * Fixed some documentation for wind speed dependence and made it so the T98 calculation will default to equatorial facing, open rack and latitude tilt. * Some documentation changes. * Update temperature.py * run pre-commit hooks * adapt wind_height variable name to NSRDB * keep wind_height in meta * refactor eff_gap_parameters() * fix typo in temperature models * workflow update Node.js 16 to 20 --------- Co-authored-by: MDKempe <58960264+MDKempe@users.noreply.github.com> Co-authored-by: Silvana Ovaitt <silvana.ayala@nrel.gov> Co-authored-by: Kempe <mkempe@nrel.gov> Co-authored-by: Silvana Ovaitt <Silvana.Ovaitt@nrel.gov> Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com> * Readthedocs pr (#61) * New Sphinx Environment * User Guide - Package Overview * Modules API * API reference * Monte Carlo User Guide * include external link aliases --------- Co-authored-by: tobin-ford <Tobin.Ford@nrel.gov> Co-authored-by: martin-springer <martinspringer.ms@gmail.com> * update what's new changelog * updates to change log * Update issue templates (#63) --------- Co-authored-by: MDKempe <58960264+MDKempe@users.noreply.github.com> Co-authored-by: Silvana Ovaitt <silvana.ayala@nrel.gov> Co-authored-by: Kempe <mkempe@nrel.gov> Co-authored-by: Silvana Ovaitt <Silvana.Ovaitt@nrel.gov> Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com> Co-authored-by: tobin-ford <145622852+tobin-ford@users.noreply.github.com> Co-authored-by: tobin-ford <Tobin.Ford@nrel.gov>
* add pypi workflow * Update and rename publish-to-test-pypi.yml to publish-to-pypi.yml * Update publish-to-pypi.yml https://github.blog/changelog/2023-12-14-github-actions-artifacts-v4-is-now-generally-available/ * Update publish-to-pypi.yml update actions/setup-python@v5 * Update publish-to-pypi.yml * Update publish-to-pypi.yml * update development (#59) * Reworking of the standoff calculations overall major reworking of the standoff calculations. Too many things to list. * standard updates * Update weather.py Just added in a print statement to indicate the humidity was finished calculating. * updates to the standoff calculation Just a bunch of updates to make it into the tool I want. * Update xeff_demo.csv I updated it to have a theoretical measured set of module data. * Update standards.py Just small formating issues here. * Create xeff_demo.xlsx * Adding calculations to the file Major revision underway. I have most of the program working but the last few sections are still needing a bit of work. * Update 4 - Standards.py Lots of reworking on the code to get it to have more methods for calculations. there is still a lot to be done. * Update weather.py Put in defaults in get_NSRDB * Update standards.py just working on it * Update weather.py Put in code to start a get_satellite function * Update weather.py typos fixed * Update weather.py fixed now. * Update weather.py Hopefully now it is patched correctly. * Update weather.py now it could work. * Update weather.py * Update weather.py fixed import nsrdbx as f * Update weather.py updated * Update 4 - Standards.ipynb update * standards * Update xeff_demo.csv Updated the demo module data to be calculated from the cell temperature instead of the module surface temperature. * Update xeff_demo.xlsx * Update xeff_demo.csv Updated the manufactured module data. * Update xeff_demo.xlsx Incldues the modeled POA data from Python and an updated module temperature calculation. * Update utilities.py I changed it to instead of just having 180 degrees south be the default azimuth, it will point north or 0 degrees by default in the southern hemisphere. * Massive changes and a stopping point It's a complete overhaul. I'm going to merge it now, but I would like it put in the main as is and to have it's name changed to "Standoff.ipynb" * Update spectral.py Making the default azimuth be equatorial facing instead of always south. No northern-centric bias here. ;) * Update temperature.py I fixed the cell temperature calculation to default to a wind speed factor of 1.7 instead of 1 and to allow that to be passed in. My guess is that this change will cause problems elsewhere. * Update standards.py Massive changes here but for the most part it is all about the standoff calculation which is working. One change to be aware of is that the prior calculations were using the module surface temperature for the standoff calculation but now it is using the cell temperature calculation. * improvement on temperature windspeedfactor fix * dding pytest coverage instructions * Wind height added Added in a field for wind height data to the demonstration weather data. * Update weather.py Added in some if/then statements to put in a field for the wind speed measurement height. For the NSRDB it is set for 2m, for PVGIS it is set for 10m. * Update weather.py Made it so it would delete the message indicating that Rh was being calculated once it is completed. * Update standards.py Created a print statement method to output the results of the standoff calculation. * Update temperature.py Changed it to do a wind speed factor calculation based on the wind height in the meta data and the supplied power factor using appropriate defaults. * Update temperature.py Got it to correctly use the wind speed exponent. * Update standards.py Got it to correctly use the wind speed exponent. * Update 4 - Standards.ipynb Got it to correctly use the wind speed exponent. * Update temperature.py Fixing wind speed stuff * Update 4 - Standards.ipynb Fixing wind speed stuff * Update fatigue.py Modified the fatigue calculator to work with the new wind speed exponent method. * Update test_fatigue.py Fixed this procedure to call the module temperature function correctly. * Update humidity.py It now uses the desired power factor for wind speed height adjustment. * Update h5_pytest.h5 It now uses the desired power factor for wind speed height adjustment. * Update meta.json It now has a field for the wind speed height. * Update test_humidity.py It now uses the desired power factor for wind speed height adjustment. * Update test_standards.py It now uses the desired power factor for wind speed height adjustment. * Update test_temperature.py It now uses the desired power factor for wind speed height adjustment. * Update standards.py It now uses the desired power factor for wind speed height adjustment. I found an error in the standoff calculation. It was previously using the module surface temperature and it should be using the module cell temperature. It makes the required standoff calculations a bit higher. * Update standards.py Fixed some documentation for wind speed dependence and made it so the T98 calculation will default to equatorial facing, open rack and latitude tilt. * Update weather.py fixed the improper importing NSRDBX as f and did some documentation work. * Update standards.py Fixed the functions with capitals issue. * Update 4 - Standards.ipynb Fixed the functions with capitals issue. * deploy to pypi on tag release (#55) * add pypi workflow * Update standards.py I fixed an error check for the case of now wind height specification in the meta data of a dataset by removing it. that error check is managed in temperature.py. * Update 4 - Standards.ipynb Just some documentation changes. * Update temperature.py just some documentation changes * run pre-commit hooks * revert changes in h5_pytest.h5 * update docs conf from main * Revert "Kempe standoff" (#57) * run updated pre-commit hooks * add pre-commit to dependencies --------- Co-authored-by: MDKempe <58960264+MDKempe@users.noreply.github.com> Co-authored-by: Silvana Ovaitt <silvana.ayala@nrel.gov> Co-authored-by: Kempe <mkempe@nrel.gov> Co-authored-by: Silvana Ovaitt <Silvana.Ovaitt@nrel.gov> * added codecov to actions * write coverage report * update pytest.yml * add pytest-cov * add build status to readme * Kempe standoff pull request (#58) * Reworking of the standoff calculations * standard updates * Update weather.py * Update xeff_demo.csv * Create xeff_demo.xlsx * Put in defaults in get_NSRDB * typos fixed * Changed standoff calculation to instead of just having 180 degrees south be the default azimuth, it will point north or 0 degrees by default in the southern hemisphere. * Update spectral.py * Massive changes here but for the most part it is all about the standoff calculation which is working. One change to be aware of is that the prior calculations were using the module surface temperature for the standoff calculation but now it is using the cell temperature calculation. * improvement on temperature windspeedfactor fix * adding pytest coverage instructions * Wind height added * Added in a field for wind height data to the demonstration weather data. * Added in some if/then statements to put in a field for the wind speed measurement height. For the NSRDB it is set for 2m, for PVGIS it is set for 10m. * Made it so it would delete the message indicating that Rh was being calculated once it is completed. * Changed it to do a wind speed factor calculation based on the wind height in the meta data and the supplied power factor using appropriate defaults. * Modified the fatigue calculator to work with the new wind speed exponent method. * Update meta.json * Update test_humidity.py * Update test_standards.py * Update test_temperature.py * Update standards.py * I found an error in the standoff calculation. It was previously using the module surface temperature and it should be using the module cell temperature. It makes the required standoff calculations a bit higher. * Update standards.py * Fixed some documentation for wind speed dependence and made it so the T98 calculation will default to equatorial facing, open rack and latitude tilt. * Some documentation changes. * Update temperature.py * run pre-commit hooks * adapt wind_height variable name to NSRDB * keep wind_height in meta * refactor eff_gap_parameters() * fix typo in temperature models * workflow update Node.js 16 to 20 --------- Co-authored-by: MDKempe <58960264+MDKempe@users.noreply.github.com> Co-authored-by: Silvana Ovaitt <silvana.ayala@nrel.gov> Co-authored-by: Kempe <mkempe@nrel.gov> Co-authored-by: Silvana Ovaitt <Silvana.Ovaitt@nrel.gov> Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com> * Readthedocs pr (#61) * New Sphinx Environment * User Guide - Package Overview * Modules API * API reference * Monte Carlo User Guide * include external link aliases --------- Co-authored-by: tobin-ford <Tobin.Ford@nrel.gov> Co-authored-by: martin-springer <martinspringer.ms@gmail.com> * update what's new changelog * updates to change log * add pull request template * Update issue templates (#63) * release notes for 0.2.3 * Monte Carlo PR (#62) * Added monte-carlo analysis package --------- Co-authored-by: tobin-ford <Tobin.Ford@nrel.gov> Co-authored-by: Silvana Ovaitt <Silvana.Ovaitt@nrel.gov> Co-authored-by: martin-springer <martinspringer.ms@gmail.com> Co-authored-by: Kempe <mkempe@nrel.gov> --------- Co-authored-by: MDKempe <58960264+MDKempe@users.noreply.github.com> Co-authored-by: Silvana Ovaitt <silvana.ayala@nrel.gov> Co-authored-by: Kempe <mkempe@nrel.gov> Co-authored-by: Silvana Ovaitt <Silvana.Ovaitt@nrel.gov> Co-authored-by: Kevin Anderson <kevin.anderso@gmail.com> Co-authored-by: tobin-ford <145622852+tobin-ford@users.noreply.github.com> Co-authored-by: tobin-ford <Tobin.Ford@nrel.gov>
I'm creating this pull request to pull in code that is ready to be included. Code snippets like in utilities are now on the Kempe_standoff_dev branch