Skip to content

Conversation

wfvining
Copy link
Collaborator

@wfvining wfvining commented Oct 5, 2020

Description

Implements method for identifying tilt and azimuth given AC power, clear sky irradiance, and solar position.

Checklist

  • Added new API functions to docs/api.rst
  • Clearly documented all new API functions with PEP257 and numpydoc compliant docstrings
  • Non-API functions clearly documented with docstrings or comments as necessary
  • Added tests to cover all new or modified code
  • Pull request is nearly complete and ready for detailed review

Implements method for identifying tilt and azimuth given ac power,
clearsky irradiance, and solar position.
Add entry to the list of functions for orientation inference in api.rst and fix minor issues in the docstring.
Test with missing data and with Series for temperature and wind_speed
parameters.
@wfvining wfvining marked this pull request as ready for review October 20, 2020 16:11
@wfvining wfvining requested a review from cwhanse October 20, 2020 16:11
Copy link
Member

@wholmgren wholmgren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great feature.

Why restrict the function to orientation when it also performs a fit over DC capacity and inverter input limit?

@wholmgren
Copy link
Member

Why restrict the function to orientation when it also performs a fit over DC capacity and inverter input limit?

Thinking about this some more, the capacity values are relatively sensitive to temperature model/coefficient, AOI model, and spectral model, while orientation is largely independent of those choices. Expanding the API to include all of those options could be a real mess, so perhaps a good idea to restrict the scope.

@wfvining
Copy link
Collaborator Author

I originally restricted to only the tilt and azimuth because I didn't know if the other parameters would be accurate (sounds like they might not be). There is, however, evidence in the literature that this type of approach can work for tilt and azimuth (Haghdadi N; Copper J; Bruce A; MacGill I, 2017, 'A method to estimate the location and orientation of distributed photovoltaic systems from their generation output data', Renewable Energy, vol. 108, pp. 390 - 400, http://dx.doi.org/10.1016/j.renene.2017.02.080). I would like to eventually push towards the method described in that paper, and this function seems like a good first step in that direction.

Series
Difference between `power_ac` and the PVWatts output with the
given parameters.
"""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""
Notes
------
Uses the defaults in `pvlib.irradiance.get_total_irradiance` to calculated plane-of-array
irradiance, i.e., the isotropic model for sky diffuse irradiance, and the Perez irradiance
transposition model.
"""

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cwhanse I don't think this code is actually using Perez. Is that something we should update? Seems like a shame to deviate from PVWatts in a way that will presumably make the fits worse.

cc @kperrynrel since she's getting bad fits :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

", and the Perez irradiance transposition model." That text is in error and should be deleted.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, should we switch the code to use Perez? I think the extra inputs could be calculated automatically. It would be more aligned with PVWatts itself, plus hopefully return better results.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't object, but on this second look we should also clarify what is meant here by 'PVWatts'. It's not referring to the PVWatts application (modelchain), but rather to the PVWatts DC and AC power functions. To that point, this pvanalytics function uses the Sandia model to get cell temperature.

If we decide we want to match PVWatts application, maybe we should use the pvlib.modelchain.ModelChain.with_pvwatts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point. Note that ModelChain.with_pvwatts currently uses the Sandia temperature model too: https://github.com/pvlib/pvlib-python/blob/master/pvlib/modelchain.py#L49-L59

I'll open an issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kanderso-nrel let me know if you need help with these edits. I put in a PR for the system documentation but I'll hold off until we resolve the issues with the functions themselves.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wfvining wfvining requested a review from cwhanse October 23, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants