Skip to content
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

Multiple arrays in a PV system #1067

Closed
cwhanse opened this issue Sep 23, 2020 · 31 comments · Fixed by #1076
Closed

Multiple arrays in a PV system #1067

cwhanse opened this issue Sep 23, 2020 · 31 comments · Fixed by #1076

Comments

@cwhanse
Copy link
Member

cwhanse commented Sep 23, 2020

I'm proposing to add capability to model more than one array in a PV system. Defining the term "array", I mean a collection of PV modules at the same orientation (tilt and azimuth). Many actual PV systems have several arrays, distinguished by different orientation and/or number of modules and strings of modules.

The current PVSystem defines the DC side of a PV system using modules_per_string and strings_per_inverter and applies attributes surface_tilt and surface_azimuth to all modules. A PV system may have a single inverter. This model implicitly describes a PV system with a single array.

@wholmgren have discussed this a bit offline and think a way forward would be to make PVSystem attributes iterable. For example, surface_tilt could contain a list or array, with each element being an array of tilts to be applied to one of the arrays comprising the PV system. PVSystem and ModelChain methods would be extended to perform calculations for each array in the PV system, broadcasting where needed.

Two principles guiding this proposal:

  1. maintain the current interface for modeling systems with a single array. It is simple but effective, and modeling a single array system shouldn't become more complicated.
  2. methods should return the type indicated by the input attributes, so that a user doesn't need to deal with lists of length 1 when modeling a single array system.

We discussed, and disfavor other approaches:

  • a new class for PV systems with multiple arrays, which not appealing when considering the duplication of code, and the likely complication of ModelChain methods.
  • subclassing PVsystem or adding attributes, either of which look to be confusing for users.

Open for comments here, before I start a branch to work on this feature.

#457 is relevant. Having multiple arrays lays the groundwork for modeling inverters with multiple MPPTs.

@cwhanse
Copy link
Member Author

cwhanse commented Sep 23, 2020

Exploring this a bit, to settle on a pattern, using a copy of the PVSystem.get_aoi method:

def get_aoi(surface_tilt, surface_azimuth, solar_zenith, solar_azimuth):
    """Get the angle of incidence on the system.

    Parameters
    ----------
    solar_zenith : float or Series.
        Solar zenith angle.
    solar_azimuth : float or Series.
        Solar azimuth angle.

    Returns
    -------
    aoi : Series
        The angle of incidence
    """

    if (isinstance(surface_tilt, int) or
        isinstance(surface_tilt, float) or
        isinstance(surface_tilt, pd.Series)):
        return irradiance.aoi(surface_tilt, surface_azimuth,
                              solar_zenith, solar_azimuth)

    aoi = [irradiance.aoi(t, a, solar_zenith, solar_azimuth)
        for t, a in zip(surface_tilt, surface_azimuth)]
    # match type of self.surface_tilt
    if isinstance(surface_tilt, np.ndarray):
        aoi = np.asarray(aoi)  # make it an array of Series?

    return aoi


from pvlib import location
loc = location.Location(latitude=35, longitude=-116)

dt = pd.date_range(start='2020-09-20 00:00:00', end='2020-09-22 00:00:00',
                   freq='1H').tz_localize('Etc/GMT+7')
sp = loc.get_solarposition(dt)

# both are int/float
pv = PVSystem()
aoi = get_aoi(pv.surface_tilt, pv.surface_azimuth, sp['zenith'], sp['azimuth'])

# both are arrays of same length
pv2 = PVSystem(surface_tilt=np.array([0., 10.]),
               surface_azimuth=np.array([180., 190.]))
aoi2 = get_aoi(pv.surface_tilt, pv.surface_azimuth, sp['zenith'], sp['azimuth'])

This runs with PVSystem.surface_tilt as an int, float, Series or array, with PVSystem.surface_azimuth of the same type and dimension. Discovered some challenges:

  1. The current method returns a Series with index taken from the input solar_zenith and solar_azimuth Series. The above function maintains that behavior if the surface_tilt is a singleton or Series with the same index as solar_zenith and solar_azimuth, but returns a 2D ndarray if surface_tilt is any other kind of iterable.
  2. PVSystem.surface_tilt can arrive as a time-indexed Series for a single array on a tracked system. This implies that we would need to restrict PVSystem.surface_tilt to surface_tilt: float or array rather than surface_tilt: float or array-like, because if surface_tilt is a Series of tilt indexed by the arrays, then the code still runs but the returned aoi is garbage (see *)
  3. this seems like a clumsy way to get around the fact that neither int nor float are interable.
  4. I don't know how to handle the broadcasting case, when e.g. surface_tilt is an array but surface_azimuth is a float, if not using a complicated sequence of if isinstance statements. If this is needed, there's got to be a better way.
  • The underlying function pvlib.irradiance.aoi_projection uses the * operator rather than the pandas multiply method. When the factors are Series with different indexes, for the product pandas merges the indexes and fills in nan for indices that aren't in both indexes. This result is not what a user would expect to see.

@wholmgren
Copy link
Member

It might simplify things if we said PVSystem could accept any iterable of tilt or azimuth but coerce it into a standard type. For example

class PVSystem:
    def __init__(surface_tilt, surface_azimuth,...):
        self.surface_tilt = np.array(surface_tilt)  # or np.array(surface_tilt).tolist()
        self.sufrace_azimuth = np.array(surface_azimuth)

The following behavior is relevant:

In [7]: np.array(1)
Out[7]: array(1)

In [8]: np.array(1).tolist()
Out[8]: 1

In [9]: np.array([1]).tolist()
Out[9]: [1]

The underlying function pvlib.irradiance.aoi_projection uses the * operator rather than the pandas multiply method. When the factors are Series with different indexes, for the product pandas merges the indexes and fills in nan for indices that aren't in both indexes. This result is not what a user would expect to see.

Two things to try:

  1. Swap the multiplication order in aoi_projection.
  2. In SingleAxisTracker.get_aoi method, change aoi(surface_tilt,...) to aoi(surface_tilt.to_numpy(),...`

@cwhanse
Copy link
Member Author

cwhanse commented Sep 24, 2020

It would simplify the typing, but I think we have to resolve the overloading of PVSystem.surface_tilt first.

  1. For a PVSystem comprising N several arrays at fixed orientation, PVSystem.surface_tilt is an iterable of N tilts, one for each array.
  2. For a PVSystem comprising N arrays with tracking, PVSystem.surface_tilt would be an iterable of length N, each element of which is a Series.

Perhaps a way forward is to use if PVSystem.axis_tilt: to decide how to resolve the ambiguous case when PVSystem.surface_tilt is a Series for a PV system with a single array with tracking.

@wholmgren
Copy link
Member

I'm a little confused...

For a PVSystem comprising N arrays with tracking, PVSystem.surface_tilt would be an iterable of length N, each element of which is a Series.

That's a SingleAxisTracker, which can and does reimplement PVSystem methods as needed to deal with these complexities.

@cwhanse
Copy link
Member Author

cwhanse commented Sep 24, 2020

Aha. PVSystem.get_aoi needs only handle fixed tilt cases, because SingleAxisTracker will handle the tracker cases. I'm tracking now.

@mikofski
Copy link
Member

mikofski commented Sep 25, 2020

  • use duck typing instead of instance checking
  • There are a bunch of broadcasting tools in NumPy, try broadcast

@ncroft-b4
Copy link
Contributor

Just as another idea at risk of disrupting this line of thought. Have you considered creating a new type of "Multi Array ModelChain" that could implement and iterate through multiple ModelChains, instead of changing anything with PVsystem? My thinking is that users who use PVsystem typically only need it for single types of arrays given it can only have a single inverter anyway, and if they need multiple sub-arrays they've already built that out of multiple PVSystems. Thus the needs for multiple sub-arrays are likely going to be ModelChain users.

A multiple array ModelChain could accept a single location, an iterable of multiple PVsystem parameters, single inputs for ModelChain parameters, and an iterable of 'inverters_per_system'. The class would then create a list of multiple PVsystem and multiple ModelChain and could have a function to run_model on a weather input that would run each of its sub_models, multiply the ac outputs for the number of inverters of each system, and sum the outputs.

The benefit here would be that all the underlying code could be left as is and I think it would be a relatively simple to implement. But this it would be more of an approach of adding a new wrapper around what is already there rather than building functionality from the bottom-up.

So it could be something like
multiarraysystem = pv.modelchain.MultiArrayModelChain(Location, system_parameters, inverters_per_array, **model_chain_parameters)

where system parameters is a list of dicts and inverters_per_array is a list of integers, and when it initializes is creates unique instances of PVsystem for each dict in the list and also creates instances of ModelChains with them. Then a function like multiarraysystem.run_model() would run the model for each sub-chain and add them together for one output.

@mikofski
Copy link
Member

@ncroft-b4 interesting idea, but I think @cwhanse is trying to address subarrays that are connected to the same inverter or to different inputs of a multi-mppt inverter (see #457 )

@mikofski
Copy link
Member

Possibly relevant to OpenPVspec and #96

@adriesse
Copy link
Member

adriesse commented Sep 26, 2020

Or how about using a subarray as a building block and creating a multi-subarray inverter?

@cwhanse
Copy link
Member Author

cwhanse commented Sep 27, 2020

@adriesse I'm not understanding your suggestion - can you elaborate?

@ncroft-b4 That's a thought... @mikofski the system I have in mind is one inverter with several non-identical DC arrays. @ncroft-b4 is right that the PVSystem attributes I'm taking up here would not address the case of a system with several inverters with different arrays.

@mikofski
Copy link
Member

Although several systems connected to different inverters is trivial, just calculate each separately. We don't have any ac post processes. So the most pressing case is the one you and I both describe:

[several] subarrays connected to the same inverter or to different inputs of a multi-mppt inverter

@adriesse
Copy link
Member

I'm not understanding your suggestion - can you elaborate?

It's hardly a fully thought-out idea. It just seems a System class could get quite complex as it grows to accommodate more and more types of systems. Perhaps a Subarray class could help to make it more modular.

@cwhanse
Copy link
Member Author

cwhanse commented Sep 28, 2020

@wholmgren and I discussed an Array class, we haven't ruled it out. I think we are hoping that iterable PVSystem attributes may do the same job.

@wholmgren
Copy link
Member

I thought that supporting iterable PVSystem.surface_tilt and PVSystem.surface_azimuth could be a relatively easy way to extend PVSystem capabilities without greatly complicating the API. I agree that the approach would quickly become complex if we extended the support to additional attributes/parameters.

Along with a new Array class, we also considered a redesign of the PVSystem class. The PVSystem redesign might be implemented as a new class rather than a modification to the existing class. If you were starting with a blank file, what would your Array and PVSystem classes look like?

@ncroft-b4
Copy link
Contributor

I do think it would need the ability to support additional attributes than just tilt and azimuth, such as module_parameters, stings_per_inverter and modules_per_string. One way to go might be to keep the PVSystem as a single building block but add an attribute that specifies that it could be a fraction of an inverter, and then two or more fractional PVSystems could make up a multi array inverter. But that assumes there is no power sharing between subarrays on the same inverter. The iterable approach being discussed would allow for power sharing, but assumes there is always power sharing and never clipping on a per-mppt basis. And as raised in #457, there is the issue of coding in a function to determine the operating dc voltage of the inverter when multiple subarrays will be tracking at different voltages.

Starting from a blank file, perhaps an Array class would wrap in all of the DC side functionality of the PVSystem, and output v_dc and p_dc for an array that is a single combo of tilt, azimuth, string size, module, and number of strings. Then I guess the PVSystem would take one or more v_dc and p_dc inputs, combine them, and contain the inverter functionality. But that leaves PVsystem as little more than the inverter functions and breaks the current interface for modelling systems.

@cwhanse
Copy link
Member Author

cwhanse commented Sep 28, 2020

Starting from a blank file, perhaps an Array class would wrap in all of the DC side functionality of the PVSystem, and output v_dc and p_dc for an array that is a single combo of tilt, azimuth, string size, module, and number of strings.

That is my thinking also, except for

breaks the current interface for modelling systems.

@ncroft-b4
Copy link
Contributor

ncroft-b4 commented Sep 28, 2020

It is a challenge to keep the current interface for PVsystem other than the iterable approach. Perhaps rebuild PVSystem to be based on one or more Array objects that it creates based on the inputs? And then each of the dc-side functions of PVSystem operate on each Array in a list of PVSystem.subarrays that replaces PVSystem.surface_tilt, PVsystem.surface_azimuth, etc.... The Arrays would have those attributes. I suppose that is just a different way of implementing the first approach and arranging the functions.

@cwhanse
Copy link
Member Author

cwhanse commented Sep 28, 2020

Instances of an Array class could be constructed by PVSystem and hidden for now, that's a thought.

@wholmgren
Copy link
Member

I opened up pvsystem.py to think about these issues and was surprised to see that the PVSystem docstring already claims to support array-like surface_tilt and surface_azimuth:

surface_tilt: float or array-like, default 0
Surface tilt angles in decimal degrees.
The tilt angle is defined as degrees from horizontal
(e.g. surface facing up = 0, surface facing horizon = 90)
surface_azimuth: float or array-like, default 180
Azimuth angle of the module surface.
North=0, East=90, South=180, West=270.

I could not find any PVSystem tests that use array-like values for these parameters. I also used github search to try to find user code that relies on array-like parameters and didn't find anything, but I didn't look very carefully.

git blame @wholmgren for this mistake 😬

@cwhanse
Copy link
Member Author

cwhanse commented Sep 28, 2020

array_like works but only when surface_tilt etc. are of the same shape as any time-series input, e.g., solar_zenith here

@wholmgren
Copy link
Member

wholmgren commented Sep 28, 2020

What about an Array with a small number of attributes:

  • surface_tilt
  • surface_azimuth
  • module_parameters
  • temperature_model_parameters
  • racking_model
  • albedo

and methods:

  • get_aoi
  • get_irradiance
  • get_iam

so that a higher level object like PVSystem can do more complicated things? I don't want to embark on a big redesign only to find that we've locked ourselves out of system design and operation that become far more common over the next 5 years (PVSystem is currently 5 years old).

Perhaps rebuild PVSystem to be based on one or more Array objects that it creates based on the inputs?
Instances of an Array class could be constructed by PVSystem and hidden for now, that's a thought.

Is the motivation for this just API compatibility with the existing PVSystem? I'm not too worried about that at this point in the discussion.

@ncroft-b4
Copy link
Contributor

Is the motivation for this just API compatibility with the existing PVSystem? I'm not too worried about that at this point in the discussion.

Yes, but also to maintain a single object that fully represents a system that continues to work with ModelChain. If Array is internal to PVSystem then I don't think anything would need to change in ModelChain, but if Array gets split from PVSystem then ModelChain will have to run both Array methods and PVSystem methods, wouldn't it?

Though if it is not too big a deal to maintain current API compatibility with PVsystem, then probably the ideal way would be to have Array, then have PVSystem accept one or more Array as input and expose the Array methods as PVsystem methods. Then maintaining ModelChain as is.

@wholmgren
Copy link
Member

probably the ideal way would be to have Array, then have PVSystem accept one or more Array as input and expose the Array methods as PVsystem methods.

This is how I'm thinking about it.

If, in addition to an Array class, we end up creating a new PVSystemVersion2 class (with a better name!) then perhaps ModelChain internals would support both PVSystem and PVSystemVersion2 for some period of time.

@cwhanse
Copy link
Member Author

cwhanse commented Sep 29, 2020

Or have PVSystem accept the inputs as iterables and PVSystem constructs the Arrays? May not be workable but preserves the current interface.

I'm happy to mock up some code for one of the Array options:

  1. new Array class, PVSystem expects an Array input, other current PVSystem inputs stay. This risks having Array and other inputs be inconsistent.
  2. new Array class, current PVSystem inputs are rolled into the Array class with deprecation.
  3. new Array class, not an input to PVSystem, but constructed from current PVSystem inputs.

Any other options to consider, and which would be your preference?

@wholmgren
Copy link
Member

Of those options, I would start with 1 or 2. It could be that we ultimately support 3 but I don't think it's worth considering as part of the initial design.

I strongly feel that we need to think about a new PVSystem class in addition to a new Array class. The existing PVSystem class is a confusing mixture of API design and forcing a new Array into this interface likely will lead to an unsatisfying local maximum in API design. By "new" PVSystem I mean a class for which it's not possible to cleanly deprecate from the current state of PVSystem into a new state. #1067 (comment) has some concrete suggestions for what the Array part could look like.

If we don't have the appetite for achieving consensus on a bigger redesign then I'd advocate for a simple solution that casts input iterables to arrays and exploits broadcasting.

@cwhanse
Copy link
Member Author

cwhanse commented Sep 29, 2020

I'm OK with a "new" and redesigned PVSystem class, but I haven't come up with a name to propose...

@adriesse
Copy link
Member

I think I like the direction the discussion is going.

The motivation for an Array class is to be able to do interesting things with more than one of them having different parameters or operating conditions. The two main cases seem to be (1) connecting them to different MPPT's on the same inverter, and (2) connecting them in parallel to the same MPPT.

(1) could be done with a new two-stage inverter efficiency model, but needs nothing new on the Array side. This doesn't seem hard to implement, but would just require some reasonable assumptions about the attribution of total internal losses. The same quadratic loss model can be used to model both conversion stages (with different parameter of course).

(2) requires combining I-V curve characteristics of the two or more arrays. This seems more well-defined, but perhaps a bit harder to implement. I would shy away from short-cuts like weighted average of Vmp though.

@cwhanse
Copy link
Member Author

cwhanse commented Oct 5, 2020

Summarizing some offline conversations for the benefit of all. It seems we are converging to the following pattern:

  • add an Array class with a small list of attributes and methods as suggested here
  • an Array instance has a single value of tilt, azimuth, etc.
  • PVSystem will have a new arrays attribute that is assigned a list of Array instances. PVSystem methods will be modified to operate on elements of PVSystem.arrays. The use of list here is deliberate so that the following works regardless of the length of self.arrays
for array in self.arrays:
  • to preserve the current interface, PVSystem.surface_tilt will remain (for a deprecation period), and PVSystem.__init__ will handle conversion of e.g. PVSystem.surface_tilt to an instance of Array assigned to PVSystem.arrays[0]. That way, old user code should still work, except for the case where a user was passing an array to PVSystem.surface_tilt and somehow getting away with it.

I still need to look uphill to ModelChain, where I'm thinking there will be substantial changes in that the attributes recording output of modeling steps may become lists e.g. ModelChain.dc is no longer a Series, it is a list of Series. Might be a more elegant way to handle that.

This pattern meets the use case of a PV system with multiple arrays, with each array connected to a separate MPPT on one system-wide inverter. Follow-on work can extend this pattern to a PV system with one inverter per array.

The case of multiple arrays feeding one inverter without separate MPPT is not addressed, but isn't ruled out by this pattern. This case would require a new function to compute DC current and voltage at the inverter input, and it's not clear how that should be done (Kirchoff's laws of course, but, one likely needs parameters that aren't already in pvlib).

@wfvining fyi

@wfvining
Copy link
Contributor

wfvining commented Oct 6, 2020

Where will Array live? Should it go in the pvlib.pvsystem module?

@cwhanse
Copy link
Member Author

cwhanse commented Oct 6, 2020

Where will Array live? Should it go in the pvlib.pvsystem module?

Good place for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants