NREL
Dev spectral irrad tools
Describe your changes
-Add function for calculating spectral front and rear side irradiance at specific wavelengths -Output separated by wavelength and by front/rear -Uses updates in bifacial_radiance (https://github.com/NREL/bifacial_radiance/pull/576) for pulling spectra from SMARTS -Uses updates in bifacialvf for sky composition method of separating sky into different components (https://github.com/NREL/bifacialvf/pull/60)
Issue ticket number and link
Fixes # (issue)
Type of change
Please delete options that are not relevant.
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
Checklist before requesting a review
- [ ] I have performed a self-review of my code
- [ ] Code changes are covered by tests.
- [ ] Code changes have been evaluated for compatibility/integration with Scenario analysis (for future PRs)
- [ ] Code changes have been evaluated for compatibility/integration with geospatial autotemplating (for future PRs)
- [ ] New functions added to init.py
- [ ] API.rst is up to date, along with other sphinx docs pages
- [ ] Example notebooks are rerun and differences in results scrutinized
- [ ] What's new changelog has been updated in the docs
@RDaxini @martin-springer I could use some help with knowing what to fix on the CI here.
I have not reviewed in detail yet. It might be worth having a call to discuss the PR, I'm unsure about a few things that are happening in it. One option to consider though, especially in relation to the extra module requirements, is creating a jupyter notebook rather than a new set of functions.
Not saying that a notebook tutorial/example is definitely the best way forward; I'm just throwing an idea out there. With a notebook, we wouldn't need to ship the bifacial packages with pvdeg, so we can avoid the additional testing and maintenance. However, we could still set up a custom testing environment for this specific notebook that does install the required modules. Users who want to run the notebook can also be prompted to install any additional requirements themselves, separately from pvdeg. At the same time, we can still ensure the notebook and the rest of pvdeg work with eachother through the testing regime, using nbval or testbook.
I have not reviewed in detail yet. It might be worth having a call to discuss the PR, I'm unsure about a few things that are happening in it. One option to consider though, especially in relation to the extra module requirements, is creating a jupyter notebook rather than a new set of functions.
Not saying that a notebook tutorial/example is definitely the best way forward; I'm just throwing an idea out there. With a notebook, we wouldn't need to ship the bifacial packages with pvdeg, so we can avoid the additional testing and maintenance. However, we could still set up a custom testing environment for this specific notebook that does install the required modules. Users who want to run the notebook can also be prompted to install any additional requirements themselves, separately from pvdeg. At the same time, we can still ensure the notebook and the rest of pvdeg work with eachother through the testing regime, using nbval or testbook.
I'm open to moving the new functions to a notebook, depends on what works best for @MDKempe and @shirubana. What about adding the bifacialvf and bifacial_radiance to project.optional-dependencies, with the imports happening only in the functions using them?
@mjprilliman let's go with the alternate solution of putting the calls inside the function, and not making all of this into a notebook
If not a notebook, I lean towards setting these as optional dependencies as it'll make it easier to set up the test workflow using the preconfigured environment with these dependencies listed
@mjprilliman, are your changes to degradation.ipynb intentional or have they just arisen from rerunning the notebook? Same for albedo.json, this file seems to have a life of its own whereby it shows up in the diff as having been changed in a PR when, in reality, it hasn't/shouldn't have been (e.g. #185)