AttributeError when using ModelChain with multiple Arrays

[cwhanse]

Describe the bug
A clear and concise description of what the bug is.

Not sure this will be either clear or concise, but: an AttributeError is generated when instantiating a ModelChain with a PVSystem that has multiple Arrays.

The AttributeError results from this line which references a PVSystem.racking_model attribute. In turn, that line is reached from here where we're trying to ensure consistency between ModelChain.temperature_model (a string) and the array temperature models (I think).

To Reproduce
See the discussion here which reports the issue.

Expected behavior
I don't see that this user made a mistake. I would not expect the AttributeError.

Versions:

• pvlib.__version__: 0.9.5

[matsuobasho]

I'd like to take a look at this but not clear from the email thread how to reproduce (i.e. namely what the systems object contains).

[cwhanse]

@matsuobasho the error results when a PVSystem has multiple arrays, but the temperature model parameters are assigned to the PVSystem rather than to each Array.

We haven't discussed what an appropriate fix would be. Ideas are welcome.

[matsuobasho]

@cwhanse , I don't have much experience with multi-array modeling so I tried to recreate the error in the post by piecing together what I saw (since it wasn't a copy-paste reproducible example). In the course of this, I encountered my own error.

import numpy as np
import pandas as pd

from pvlib import pvsystem
from pvlib.location import Location
from pvlib.modelchain import ModelChain
from pvlib.temperature import TEMPERATURE_MODEL_PARAMETERS

arr_num = 3

target_mods = cec_modules.iloc[:,np.random.randint(cec_modules.shape[1], size=arr_num)].columns
name = ['arr_'+ str(i) for i in range(arr_num)]
azimuth = [180, 200, 210]
strings = [5, 10, 8]
tilt = [20, 25, 29]
mods_per_string = [12, 14, 16]

arrays = pd.DataFrame({'name': name,
                      'module': target_mods,
                      'azimuth': azimuth,
                      'strings': strings,
                      'tilt': tilt,
                      'modules_per_string': mods_per_string})

temp_params = TEMPERATURE_MODEL_PARAMETERS["pvsyst"]["freestanding"]

location = Location(latitude=32.2, longitude=-110.9)

pv_arrays = []
for i in range(len(arrays)):
    mount = pvsystem.FixedMount(surface_tilt=arrays.iloc[i]["tilt"], surface_azimuth=arrays.iloc[i]["azimuth"])
    module = cec_modules[arrays.iloc[i]["module"]]

    pv_arrays.append(
        pvsystem.Array(
            mount=mount,
            module_parameters=module,
            strings=arrays.iloc[i]["strings"],
            modules_per_string=arrays.iloc[i]["modules_per_string"],
            name=arrays.iloc[i]["name"],
        )
    )

    # create the PV system
    inverter = cec_inverters.iloc[:,1000]  # random inverter for testing
    pv_system = pvsystem.PVSystem(
        arrays=pv_arrays,
        inverter_parameters=inverter,
        temperature_model_parameters=temp_params,
        name='dummy_name',
    )

    # create the model chain
    modelchain = ModelChain(
        pv_system, location, name='dummy_name', aoi_model="physical", temperature_model="sapm"
    )

The error comes on the ModelChain instantiation:
AttributeError: 'PVSystem' object has no attribute 'racking_model'

I see this error is referenced here, but not really following what role Feedinlib plays, since it doesn't seem to be a library that's called by pvlib. I've pulled the latest version of PVLib from GH source.

Please help on this one and I'll proceed to investigate possible solutions for the issue at hand. Thanks.

[cwhanse]

@matsuobasho same line, different outcome. In your example, pv_system.racking_model isn't set, so the error results because it is referenced.

The offending logic is in this if:

        # remove or statement in v0.9
        if {'a', 'b', 'deltaT'} <= params or (
                not params and self.system.racking_model is None
                and self.system.module_type is None):
            return self.sapm_temp
        elif {'u_c', 'u_v'} <= params:

Since v0.9 we said we would require temperature_model_parameters, which is (I believe) the intent of that comment: a reminder (that we neglected) to remove code that would default to the SAPM temperature model when none of temperature parameters, racking model or module type are set.

If we remove the or part, then it would fix the error you found, and may solve the error when temperature_model_parameters aren't assigned to the arrays.

[matsuobasho]

@cwhanse , thanks for the clarification, understood.

Is there an issue with passing temp_params to the temperature_model_parameters argument for each array? If I delete the entire or clause from here like you had suggested, and then pass the temp_params to each array, then it works:

import numpy as np
import pandas as pd

from pvlib import pvsystem
from pvlib.location import Location
from pvlib.temperature import TEMPERATURE_MODEL_PARAMETERS
from pvlib.modelchain import ModelChain

cec_modules = pvsystem.retrieve_sam('CECMod')
cec_inverters = pvsystem.retrieve_sam('cecinverter')

arr_num = 3

target_mods = cec_modules.iloc[:,np.random.randint(cec_modules.shape[1], size=arr_num)].columns
name = ['arr_'+ str(i) for i in range(arr_num)]
azimuth = [180, 200, 210]
strings = [5, 10, 8]
tilt = [20, 25, 29]
mods_per_string = [12, 14, 16]

arrays = pd.DataFrame({'name': name,
                      'module': target_mods,
                      'azimuth': azimuth,
                      'strings': strings,
                      'tilt': tilt,
                      'modules_per_string': mods_per_string})

temp_params = TEMPERATURE_MODEL_PARAMETERS["pvsyst"]["freestanding"]

location = Location(latitude=32.2, longitude=-110.9)

inverter = cec_inverters.iloc[:,1000]  # random inverter for testing

pv_arrays = []
for i in range(len(arrays)):
    mount = pvsystem.FixedMount(surface_tilt=arrays.iloc[i]["tilt"], surface_azimuth=arrays.iloc[i]["azimuth"])
    module = cec_modules[arrays.iloc[i]["module"]]

    pv_arrays.append(
        pvsystem.Array(
            mount=mount,
            module_parameters=module,
            strings=arrays.iloc[i]["strings"],
            modules_per_string=arrays.iloc[i]["modules_per_string"],
            name=arrays.iloc[i]["name"],
            temperature_model_parameters = temp_params
        )
    )

pv_system = pvsystem.PVSystem(
    arrays=pv_arrays,
    inverter_parameters=inverter,
    temperature_model_parameters=temp_params,
    name='dummy_name',
)

# create the model chain
modelchain = ModelChain(
    pv_system, location, name='dummy_name', aoi_model="physical", temperature_model="pvsyst"
)

If this is acceptable, then it should be just a matter of specifying in the PVSystem documentation and tutorial that each array requires its own temp_params, even if all arrays have the same ones.

Offering the simplest solution that comes to mind, quite possible I'm not grasping the extent of the problem.

[cwhanse]

@matsuobasho the goal is that ModelChain works with temperature_model_parameters assigned to each Array, with one or more Arrays. For compatibility purposes, ModelChain should also work with a PVSystem that doesn't have an Array, since we haven't yet required an Array when constructing the PVSystem. Thus the gap that generated the first error: a user can assign temperature_model_parameters to a PVSystem and also assign Arrays. Since it is possible to do that, as a user I would also assume that pvlib would handle the placement of temperature_model_parameters (which is doesn't).

The short path to a fix is to do as you describe: make the documentation clear that temperature_model_parameters are assigned to Arrays when Arrays are used. An optional addition would be to help a user by checking if both PVSystem.Arrays and PVSystem.temperature_model_parameters are assigned, and if so, copy the parameters to the Arrays and then remove the parameters from PVSystem.

[matsuobasho]

I realized that my previous formulation has a redundancy that is confusing to a user.

Namely, I assign the temperature_model_params both in the PVSystem instantiation, as well as each instantiation of an array. What if we changed the PVSystem __init__ method so that the temperature_model_parameters argument has to be None if there is an arrays argument. So if arrays is provided, then we have an assert statement checking that temp_model_params isn't passed, like below in the PVSystem definition.

def __init__(self,
    arrays=None,
    surface_tilt=0, surface_azimuth=180,
    albedo=None, surface_type=None,
    module=None, module_type=None,
    module_parameters=None,
    temperature_model_parameters=None,
    modules_per_string=1, strings_per_inverter=1,
    inverter=None, inverter_parameters=None,
    racking_model=None, losses_parameters=None, name=None):
    
    if (arrays is not None) & (temperature_model_parameters is not None):
        raise ValueError("If using arrays argument, the "
                                    "temperature_model_parameters must be null.")

I propose that over checking if both are assigned to make it explicit to the user that the argument's use is mutually exclusive to either passing it to a system, or to arrays when there are multiple arrays.

Also, is it fine to just delete the or part of infer_module_temperatures like you had stated above referencing here, or do we need to investigate options to resolve this?

[cwhanse]

I propose that over checking if both are assigned to make it explicit to the user that the argument's use is mutually exclusive to either passing it to a system, or to arrays when there are multiple arrays.

I don't think that check is needed. If we did that for temperature_model_parameters we ought to do it for all the other attributes that can be assigned to Array or to PVSystem, e.g., surface_tilt. The PVSystem docstring already contains "If arrays is
specified the following PVSystem parameters are ignored:" perhaps that statement could be made emphatic.

[cwhanse]

I vote to simply delete the or clause, which will prevent the error we've seen.

[matsuobasho]

I deleted the or clause and then in the course of testing noticed something interesting and relevant.

When we create an array if the temperature_model_parameters argument is None, then we run _infer_temperature_model_params in the __init__ of Array.

When we pass the arguments to _infer_temperature_model_params without specifying temp_params manually for every single array as in my original example from Dec 11, the param_set object is 'None_None' because self.mount.racking_model and self.module_type are None.

   def _infer_temperature_model_params(self):
        # try to infer temperature model parameters from racking_model
        # and module_type
        param_set = f'{self.mount.racking_model}_{self.module_type}'
        if param_set in temperature.TEMPERATURE_MODEL_PARAMETERS['sapm']:
            return temperature._temperature_model_params('sapm', param_set)
        elif 'freestanding' in param_set:
            return temperature._temperature_model_params('pvsyst',
                                                         'freestanding')
        elif 'insulated' in param_set:  # after SAPM to avoid confusing keys
            return temperature._temperature_model_params('pvsyst',
                                                         'insulated')
        else:
            return {}

This leads to returning an empty dictionary, and eventually the error when this pv_system with arrays is passed to ModelChain. So shouldn't we resolve this racking_model/module_type issue? The way Array is written, it appears to be intended to automatically infer the temperature parameters and assign it to the array if they're not specified by the user.

[cwhanse]

Regarding temperature_model_parameters, racking_model and module_type, four cases should work:

1. the user assigns temperature_model_parameters to each Array.
2. the user assigns temperature_model_parameters to PVSystem with PVSystem.Arrays = None. In this case, PVSystem creates an Array and assigns temperature_model_parameters to it.
3. the user assigns racking_model and module_type when creating each Array (racking_model is assigned to Mount). In this case, Array instantiation tries to infer temperature_model_parameters
4. the user assigns both racking_model and module_type to PVSystem. PVSystem creates an Array and see case 3.

Any other case should result in an error, because there isn't enough information for a ModelChain to know which temperature model to run. I think the example you describe should error (temperature_model_parameters, racking_model and module_type are all None).

We could check if temperature_model_parameters is an empty dict after these lines and raise a more informative error at that point.

[matsuobasho]

Thanks for that breakdown.

So something like this in Array:

if temperature_model_parameters is None:
    self.temperature_model_parameters = \
        self._infer_temperature_model_params()
else:
    self.temperature_model_parameters = temperature_model_parameters

if temperature_model_parameters is None:
    raise ValueError("The `temperature_model_parameters` is empty "
                    "after an attempt to infer it.  "
                    "Pass the following arguments either to `Array` "
                    "or to `PVSystem`: "
                    "`temperature_model_parameters` "
                    "or `racking_model` and `module_type`.")

Also, I think it would be be beneficial to add your breakdown to the PV System and Arrays section of the PVSystem tutorial.

Finally, would this modification call for writing new tests?

Let me know if the proposed modifications make sense and I'll proceed.

[cwhanse]

That message looks fine. I'd put a comma before "or racking_model..."

We would want a new test, that simply checks that the error is raised. Here's a pattern to follow.

[matsuobasho]

As I was writing and testing, I realized the message needs to be more informative to spell out the 3 options explicitly to get rid of the error:

if self.temperature_model_parameters is None:
    raise ValueError("The `temperature_model_parameters` is empty "
                    "after an attempt to infer it.  "
                    "Pass either `temperature_model_parameters` to "
                    "`Array` or `PVSystem` (if not passing arrays), or "
                    "`racking_module` to the `Array` `mount` "
                    "object and `module_type` to `Array.")

However, the code also runs without fail when we don't pass module_type.

temp_params = TEMPERATURE_MODEL_PARAMETERS["pvsyst"]["freestanding"]
arr_num = 3

target_mods = cec_modules.iloc[:,np.random.randint(cec_modules.shape[1], size=arr_num)].columns
name = ['arr_'+ str(i) for i in range(arr_num)]
azimuth = [180, 200, 210]
strings = [5, 10, 8]
tilt = [20, 25, 29]
mods_per_string = [12, 14, 16]

arrays = pd.DataFrame({'name': name,
                      'module': target_mods,
                      'azimuth': azimuth,
                      'strings': strings,
                      'tilt': tilt,
                      'modules_per_string': mods_per_string})

location = Location(latitude=32.2, longitude=-110.9)

# create the PV system
inverter = cec_inverters.iloc[:,1000]

pv_arrays = []
for i in range(len(arrays)):
    mount = pvsystem.FixedMount(surface_tilt=arrays.iloc[i]["tilt"], surface_azimuth=arrays.iloc[i]["azimuth"], racking_model='freestanding')
    module = cec_modules[arrays.iloc[i]["module"]]

    pv_arrays.append(
        pvsystem.Array(
            mount=mount,
            module_parameters=module,
            strings=arrays.iloc[i]["strings"],
            modules_per_string=arrays.iloc[i]["modules_per_string"],
            name=arrays.iloc[i]["name"],
        )
    )

pv_system = pvsystem.PVSystem(
    arrays=pv_arrays,
    inverter_parameters=inverter,
    temperature_model_parameters=temp_params,
    name='dummy_name'
)

# create the model chain
modelchain = ModelChain(
    pv_system, location, name='dummy_name', aoi_model="physical", temperature_model="pvsyst"
)

That's because when we get to _infer_temperature_model_params, param_set = 'freestanding_None' from the example above, so we go into the freestanding condition where the temperature is pulled from temperature._temperature_model_params.

Seems like different racking_models and model_types would potentially error out, not sure. But seems like we do need to identify the possible combinations in order to write the proper recommendation to the user in the error message in question.

[cwhanse]

racking_model='freestanding' without module_type should work if temperature_model='pvsyst' but should error if temperature_model is anything else. I'm OK requesting module_type for all cases, in the interest of simplicity.

[matsuobasho]

Ok. Is it sufficient to create the test that the error is raised with one non-pvsyst temperature_model, or is the standard practice to create the tests for multiple or all versions?

[cwhanse]

We can parameterize the test so that it loops over combinations of temperature_model, racking_model and module_type. It is only necessary to check that the error is raised. Let me know if you'd like help with setting up the test that way.

[matsuobasho]

I'll give it a shot, will post what I come up with. Thanks.

[matsuobasho]

Here's my version, which I intend to place right before test_Array__infer_temperature_model_params in test_pvsystem.py:

@pytest.mark.parametrize("racking_model, module_type",
                         [(None, None),
                          (None, 'glass_polymer'),
                          (None, 'glass_glass'), 
                          ('open_rack', None),
                          ('close_mount', None),
                          ('close_mount', 'glass_polymer'),
                          ])
def test_Array_temperature_model_params_error(racking_model, module_type):
    mount = pvsystem.FixedMount(surface_tilt=20, surface_azimuth=180,
                                racking_model=racking_model)
    with pytest.raises(ValueError, match='`temperature_model_parameters` is empty'):
        pvsystem.Array(mount = mount, module_type=module_type)

A couple of points:

1. I included the condition when racking is 'close_mount' and module is 'glass_polymer' in the test because an error is raised with this combination. The param_set in _infer_temperature_model_params (close_mount_glass_polymer) doesn't satisfy any conditions and returns an empty dictionary. Perhaps this combination is impossible to infer a temperature model from and intended, but seems like we may need to at least note this in the PVSystem tutorial, if not mention it in the error message explicitly.
2. insulated_back seems to behave like freestanding in that it doesn't error out when the module_type isn't included. So it's not in my combination of tests. This seems like expected behavior?
3. I noticed that the FixedMount class docstring doesn't include freestanding as one of the options for the racking_model. I realize this is a separate issue, can open one if necessary.
4. You had written

racking_model='freestanding' without module_type should work if temperature_model='pvsyst' but should error if temperature_model is anything else. I'm OK requesting module_type for all cases, in the interest of simplicity.

But temperature_model is not explicitly specified when we create Array, just inferred. I think that's what you had meant, but just making sure.

5. I'm new to the effect of the racking_model and module_type on the type of temperature model that's selected. Are there details of this in the documentation? At a high level, would like to understand why the pvsyst model requires just the racking type, while the sapm model requires both?

[cwhanse]

The test parameters look OK to me.

The combinations of racking_model and module_type that have built-in parameters are here. The names are legacy from a Sandia report and earlier versions of pvlib.

1. 'close_mount_glass_polymer' should error, as there is no parameter set for this combination. That traces back to the Sandia report not addressing this combination.
2. 'insulated_back' doesn't error with no module_type because 'insulated' is a named parameter set for the PVSyst model, and the logic for selecting a parameter set uses if 'insulated' in , so if racking_model='insulated_back' the result will be the PVSyst model with the 'insulated' parameter set. I think this is OK.
3. Good point, that's an omission. That docstring should also mention the PVSyst temperature model options. New issue, I think.
4. You are correct: the PVSystem or Array is created before ModelChain, and its ModelChain that wants temperature_model='pvsyst' or similar.
5. Details on the use of racking_model and module_type are unfortunately not well-explained. This could be added as a Note to the PVSystem and Array docstrings. Same new issue as for 3, I think.

[matsuobasho]

Since we are now requiring temperature_model_parameters explicitly or implicitly, all places where we didn't have that both in the tutorials and the tests need to be modified.

1. I updated the PVSystem tutorial accordingly by adding the required parameters where applicable. I also updated the 'PVSystem attributes' section since it's not just tilt and azimuth that are required, but also the temperature model parameters. Open to feedback on it now or once I submit the PR.

2. As I mentioned, around 20 tests that instantiate pvsystem.PVSystem now fail. It seems that the way to proceed is to create a temperature_model_parameters fixture in conftest.py to pass to these tests. Perhaps some of these tests will be more nuanced and a fixture won't do. Let me know if this makes sense.
   A bit of a naive question, is there a more efficient way of fixing these tests aside from just going through them manually? Seems like some sort of automated code-pair tools could exist to speed up this process.

3. Also, as we discussed, I removed these lines in infer_temperature_model. The test_infer_temp_model for this function passes. But just want to make sure that we wouldn't need to modify it.

[cwhanse]

Thanks for this effort, more than I anticipated :) I look forward to seeing the PR.

[matsuobasho]

1. Should the temp_model_parameters fixture be in conftest.py or test_pvsystem.py? I'm leaning towards the latter, but maybe it'll be used in other module tests?
2. I'm passing the same temp_model_parameters fixture to this pvwatts_system_defaults fixture I'm using everywhere else:
   TEMPERATURE_MODEL_PARAMETERS["pvsyst"]["freestanding"]
{'u_c': 29.0, 'u_v': 0}
   However, perhaps this is not the correct parameter for the PVWatts system?
3. Is test_Array__infer_cell_type still testing what we want after I add the temperature_model_parameters?
4. In test_PVSystem_multi_array___repr__ , for the temperature_model_parameters in lines 2155 and 2165, do I hardcode whatever parameters I have set in in the fixture? If we change the fixture for some reason this would also have to be changed.
5. Looks like test_Array_temperature_missing_parameters needs to be modified somehow or it might even no longer be needed?

[cwhanse]

@matsuobasho at this point I think it would be easier if you could submit a PR, then we can see how the code and test are changed.

[matsuobasho]

@cwhanse , opened a PR as per your suggestion

[kandersolar]

Sorry for not chiming in earlier. I was a bit confused about what exactly the problem was here, but I think I have a clear view now, and I think . Sorry in advance for the long comment...

There are a two distinct issues here:

ModelChain sometimes accesses a non-existent attribute of PVSystem (the title of this issue) and causes AttributeError.

This occurs when the user makes a mistake and doesn't supply temperature_model_parameters, or racking_model and module_type, in the Arrays when creating and using a PVSystem for temperature modeling with ModelChain. There is no way for pvlib to proceed in this case, so raising an error is correct. The only problem is that, due to an oversight made in v0.10.0, the specific error message is confusing in pvlib v0.10.*:

• v0.9.5: KeyError: "Missing required parameter 'a'. Found {} in temperature_model_parameters."
  • plus warnings like pvlibDeprecationWarning: The PVSystem.racking_model function was deprecated in pvlib 0.9 and will be removed in 0.10. Use PVSystem.arrays[i].racking_model instead.
• v0.10.3: AttributeError: 'PVSystem' object has no attribute 'racking_model'

I think resolving this is a simple bugfix: fix the error message (and type) being raised. No deprecation period needed or desired.

ModelChain has (kind of) retained the deprecated behavior of defaulting to the SAPM cell temperature model when parameters aren't supplied and available for inference.

This was supposed to be removed in v0.9, but it was overlooked. I think resolving this problem is also a simple bugfix, not a breaking change: the behavior in question only applies when no parameters are supplied, and it's not like the model can be run in that situation anyway.

---

Anyway, I think both of these problems can be solved by just removing the second half of that offending or condition in ModelChain. I don't see why any further modifications are needed in order to fix those problems. In particular, I'm opposed to the idea of raising an error in the Array constructor when the temperature model parameters weren't specified and couldn't be inferred. Valid uses of Array/PVSystem that have no need for temperature_model_parameters include:

• Using Array for tasks unrelated to temperature modeling, e.g. irradiance transposition
• Using ModelChain with cell_temperature included in the input weather data

In cases like these, the user shouldn't be burdened and confused by making them specify a parameter set that won't get used anyway. I think what we want is for un-inferrable parameters to only cause an error when the user runs something that actually needs those parameters.

[matsuobasho]

I think you answer this above, but there quite a few combinations for me to wrap my head around this. I don't specify the temperature model or the racking model in the code below (after removing the or condition in ModelChain).

import numpy as np
import pandas as pd

from pvlib import pvsystem
from pvlib.location import Location
from pvlib.modelchain import ModelChain

cec_modules = pvsystem.retrieve_sam('CECMod')
cec_inverters = pvsystem.retrieve_sam('cecinverter')

arr_num = 3

target_mods = cec_modules.iloc[:,np.random.randint(cec_modules.shape[1], size=arr_num)].columns
name = ['arr_'+ str(i) for i in range(arr_num)]
azimuth = [180, 200, 210]
strings = [5, 10, 8]
tilt = [20, 25, 29]
mods_per_string = [12, 14, 16]

arrays = pd.DataFrame({'name': name,
                      'module': target_mods,
                      'azimuth': azimuth,
                      'strings': strings,
                      'tilt': tilt,
                      'modules_per_string': mods_per_string})

location = Location(latitude=32.2, longitude=-110.9)

# create the PV system
inverter = cec_inverters.iloc[:,1000]  # random inverter for testing

pv_arrays = []
for i in range(len(arrays)):
    mount = pvsystem.FixedMount(surface_tilt=arrays.iloc[i]["tilt"], surface_azimuth=arrays.iloc[i]["azimuth"])
    module = cec_modules[arrays.iloc[i]["module"]]

    pv_arrays.append(
        pvsystem.Array(
            mount=mount,
            module_parameters=module,
            strings=arrays.iloc[i]["strings"],
            modules_per_string=arrays.iloc[i]["modules_per_string"],
            name=arrays.iloc[i]["name"],
            module_type='glass_glass'
        )
    )

pv_system = pvsystem.PVSystem(
    arrays=pv_arrays,
    inverter_parameters=inverter,
    name='dummy_name'
)

# create the model chain
modelchain = ModelChain(
    pv_system, location, name='dummy_name', aoi_model="physical", temperature_model="sapm"
)

I get an error on ModelChain creation:
ValueError: could not infer temperature model from system.temperature_model_parameters. Check that all Arrays in system.arrays have parameters for the same temperature model. Common temperature model parameters: set().
Is this the desired behavior? Error message seems cryptic to me without additional explanation since I didn't specify temperature parameters in arrays.

Also, is the error in the right place here - in other words, pvsystem is fine, but ModelChain is not?

[kandersolar]

Is this the desired behavior? Error message seems cryptic to me without additional explanation since I didn't specify temperature parameters in arrays.

Yes, this is the desired behavior. Doesn't Check that all Arrays in system.arrays have parameters for the same temperature model. tell you what you need to do? Checking pv_system.arrays shows that the arrays all have empty dicts, so clearly they don't have parameters for the same temperature model :)

Of course, suggestions for clarifying that error text are welcome! But that's a separate issue from this one.

Also, is the error in the right place here - in other words, pvsystem is fine, but ModelChain is not?

IMHO yes, this makes sense. With the pv_system in your example code, we can still do non-temperature things like calling pv_system.get_irradiance(...). PVSystems are still useful for many things even when you don't specify temperature model parameters.

[matsuobasho]

@kandersolar , that explanation makes sense, thanks.

Earlier in this thread, @cwhanse wrote the following:

@matsuobasho the goal is that ModelChain works with temperature_model_parameters assigned to each Array, with one or more Arrays. For compatibility purposes, ModelChain should also work with a PVSystem that doesn't have an Array, since we haven't yet required an Array when constructing the PVSystem. Thus the gap that generated the first error: a user can assign temperature_model_parameters to a PVSystem and also assign Arrays. Since it is possible to do that, as a user I would also assume that pvlib would handle the placement of temperature_model_parameters (which is doesn't).

So note what happens in the following scenario where we're not specifying arrays manually:

modules = pvsystem.retrieve_sam('cecmod')

module_parameters = modules['Canadian_Solar_Inc__CS5P_220M']

inverter_parameters = {'pdc0': 5000, 'eta_inv_nom': 0.96}

system = pvsystem.PVSystem(inverter_parameters=inverter_parameters,
                           module_parameters=module_parameters)

# create the model chain
modelchain = ModelChain(
    system, location, name='dummy_name', aoi_model="physical", temperature_model="pvsyst"
)

We get the same error as in my previous example:
ValueError: could not infer temperature model from system.temperature_model_parameters. Check that all Arrays in system.arrays have parameters for the same temperature model. Common temperature model parameters: set().

Maybe the behavior is correct but the second sentence in this error message is definitely off since I understand we want to be able to instantiate PVSystem without specifying arrays.

[kandersolar]

Maybe the behavior is correct but the second sentence in this error message is definitely off since I understand we want to be able to instantiate PVSystem without specifying arrays.

Fair! Want to open a new issue to discuss clarifying that message? Note that this same kind of message is used in several places in ModelChain (open modelchain.py and search for the text "could not infer"), so if we change it in one spot, it probably makes sense to change everywhere. Either way, it's a separate issue from this one.

[matsuobasho]

Yes, #1946