Determine whether a system has a fixed orientation or is equipped with a tracker #49
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closed
wfvining
force-pushed
the
system-orientation
branch
from
53f23f9
to
f7d1d79
Compare
- Use an enum for the return value, rather than a string. - Calculation of envelopes that will be used for curve fitting.
Trying to take just the core functionality form PVFleets QA. These are the main differences: 1. Data is not separated into winter and summer months. This means that if there are time-shifts in the data they could mess up the fit. By removing the separation we get the core functionality (curve fitting) and can leave it to the user to pass only winter/summer and combine the orientation inferred for each in to a meaningful result (i.e. PVFleets basically decides the orientation is inconclusive if the winter and summer don't match) 2. A Boolean mask for day/night is passed as a parameter 3. Clipping is passed as a Boolean mask, and the percent of clipping is calculated (rather than passing in clip_percent directly) 4. The responsibilities of the function have been made narrower, reducing the number of values returned and hopefully making future maintenance simpler. It is possible (probable?) that I went too far in removing responsibilities and that some additional return values are needed. When this comes up we should look carefully at whether to add them here or add new functions with separate responsibilities. Going forward I need to make a testing plan for this and write some tests. Also need to decide whether returning None is a good idea, or if I should add a third instance to the Orientation Enum (e.g. INDETERMINANT). Finally I would like to do a bit of simplification in the _orientation_from_fit function. It would be nice if this could be made more general/flexible (i.e. pass in the various thresholds), it will take a bit of exploring to figure out the best way forward for this.
Change name of first parameter and update description of several functions and their parameters.
The orientation of a GHI sensor is FIXED.
previously was passing the polynomial object, linregress needs the y values
enum.auto() is not supported on python 3.5. We add the @enum.unique decorator to enfore uniqueness of instances since the values are now assigned manually.
Rather than abusing/overloading None, we should explicitly signal that the orientation could not be determined from the data.
This change is a first step towards letting the user pass in a data structure with their own minimum r^2 values. Not quite there, but this makes it so that the when we do pass in these parameters we don't have to change the Tracking/Fixed/Unknown logic.
Uses pvlib to simulate power from a fixed PV system and verifies that system.orientation() returns Orientation.FIXED.
Calculating clearsky separately for every test that needs it slows the tests down. Here I change the scope of the summer_clearsky fixture so it will be computed only once. I also change the timestamp spacing from 10 minutes to 1 hour, substantially speeding up the tests. Not a huge performance difference right now, but as more tests are added small performance gains should add up.
The minimum r-squared for curve fits changes depending on the amount of clipping in power data passed in. Originally the different minimums were hard coded, this commit is a first attempt to let users pass in their own minimums. It needs substantially more documentation.
simplified_solis does not require tables.
wfvining
force-pushed
the
system-orientation
branch
from
f7d1d79
to
7a882c4
Compare
Use functions in 'util._fit' and 'util._group' rather than locally defined functions.
Defines _is_tracking/_is_fixed predicates which shortens lines and makes the code more clear.
If the profile of the median does not match the profile of the 99.5% quantile (because 100 days have garbage data) then the orientation is UNKNOWN.
Zero slope (in the constant data) leads to warnings from the curve fitting funcitons due to division by zero.
100 days of garbage data messes up the median and causes the fit to fail when 'fit_median' is True.
- Don't need to test POA separately. - Require timezone localized series as input.
Basically we can classify fixed as tracking by messing with the thresholds.
Substantially expanded the documentation in the docstring
Update _infer_tracking() function to pass x and y params to _fit functions.
Instead of passing clipping percentage and maximum clipping percent down through three layers of internal functions we perform the clipping percent test and get the fit bounds at the top level. This makes the code somewhat more efficient by short-circuiting the resampling/grouping steps if there is too much clipping. The number of parameters that are threaded through the internal functions is also reduced by two, which should improve maintainability and readability.
Use the ratio of clipped measurements to the total number of measurements directly rather than converting to a percent. With a fraction between 0 and 1 the fit params dicitonary is a little easier to understand.
Users can pass in a tuple with the summer months and winter months specified in lists. This gives users in different regions the ability to customize the performance of the algorithm to match their particular seasons.
Make it clear that if there is too much clipping neither TRACKING nor FIXED can be determined.
cwhanse
approved these changes
Aug 25, 2020
Reorder the params for system.is_tracking_envelope() so that parameters relating to the envelope fit are together and parameters relating to the median fit are together.
Substantial improvements to documentation from code review.
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
We can use any type for the "list" of months, as long as it is iterable. Changed to a tuple to avoid problems inherent in using mutable objects as default values.
cwhanse
reviewed
Aug 28, 2020
Add coverage for various corner-cases and for disabling the seasonal split all together.
Change the semantics of the seasonal_split param. Now expects a dictionary with keys 'winter' and 'summer'. The behavior of `infer_tracking_envelope()` is also updated so if either season has no data, curve fitting proceeds only on the season that has data (previously the entire series was used).
The `winter_perturbed` fixture did not perturb enough data to dramatically change the median. Extends the months that are perturbed into Spring so that if the entire year is used for curve fitting the median fit will fail.
We just return UNKNOWN in this case; however, it is possible that a user made a mistake in this case. Providing a warning seems like the polite thing to do.
Highlight that seasonal grouping is optional and improve description of `seasonal_split` parameter.
|
@cwhanse I implemented the optional seasonal split with the dictionary parameter discussed above (along with new tests). Would you mind taking a quick look to see if you think it is reasonable? I didn't want to put a dictionary as the default value directly (mutability concerns) so I used a descriptive key-word. I also added a warning for possible user error when neither season has data (seems more polite than a ValueError, or silently returning UNKNOWN). |
cwhanse
reviewed
Aug 31, 2020
|
Looks good you decide about that last comment. |
wfvining
commented
Aug 31, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a function to the
pvanalytics.systemmodule for identifying the orientation (Fixed or Tracking) of a PV system from power or irradiance data. This is a fairly early draft, but I wanted to get it out there for early feedback/suggestions.This is derived from the PVFleets QA Analysis project.
(incomplete) To Do list
tests/test_system.py)