Skip to content

Resolving AttributeError when using ModelChain with multiple Arrays #1938

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.

Sign up for GitHub

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

Closed
wants to merge 16 commits into from
Closed

Resolving AttributeError when using ModelChain with multiple Arrays #1938

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
37ddf59
Delete racking_model check in infer_temperature_model of model_chain
matsuobasho Dec 15, 2023
dde73ee
Merge branch 'main' of github.com:matsuobasho/pvlib-python into 1759-…
matsuobasho Dec 15, 2023
d1b3d80
In pvsystem __init__ raise error if temperarure_model_parameters is s…
matsuobasho Dec 18, 2023
fa0c606
Grammatical typo correction in ValueError in null temp_model_params c…
matsuobasho Dec 18, 2023
8bcbaa7
Merge branch 'pvlib:main' into main
matsuobasho Dec 18, 2023
2f4ada3
Updated error message when temperature_model_parameters is null after…
matsuobasho Dec 19, 2023
13a5bc4
Add test for raising value error when instantiating array without req…
matsuobasho Dec 20, 2023
8107130
Correct if condition in checking temperature_model_parameters after i…
matsuobasho Dec 20, 2023
2bafd68
Added some additional details to the error message to make it informa…
matsuobasho Dec 20, 2023
5f2abdf
Add raises section in docstring of Array to describe error
matsuobasho Dec 21, 2023
72bf8f0
Update pvsystem user guide to reflect requirement for temperature mod…
matsuobasho Dec 21, 2023
196be6a
Update instances of pvsystem instantiation in test_pvsystem tests to …
matsuobasho Dec 22, 2023
013b1a2
Add temp_model_params fixture to test_pvsystem.py
matsuobasho Dec 22, 2023
fd5eb63
Merge branch 'pvlib:main' into main
matsuobasho Dec 22, 2023
fe40f1b
Update what's new
matsuobasho Dec 22, 2023
2e02c49
Added temp_model_parameters to pvsystem instantiation in Contributing…
matsuobasho Jan 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 45 additions & 17 deletions docs/sphinx/source/user_guide/pvsystem.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,15 @@ that describe a PV system's modules and inverter are stored in

.. ipython:: python

from pvlib.temperature import TEMPERATURE_MODEL_PARAMETERS

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

module_parameters = {'pdc0': 5000, 'gamma_pdc': -0.004}
inverter_parameters = {'pdc0': 5000, 'eta_inv_nom': 0.96}
system = pvsystem.PVSystem(inverter_parameters=inverter_parameters,
module_parameters=module_parameters)
module_parameters=module_parameters,
temperature_model_parameters = temp_params)
print(system.inverter_parameters)


Expand Down Expand Up @@ -125,7 +130,8 @@ passed to `PVSystem.module_parameters`:
module_parameters = {'pdc0': 5000, 'gamma_pdc': -0.004}
inverter_parameters = {'pdc0': 5000, 'eta_inv_nom': 0.96}
system = pvsystem.PVSystem(module_parameters=module_parameters,
inverter_parameters=inverter_parameters)
inverter_parameters=inverter_parameters,
temperature_model_parameters=temp_params)
print(system.arrays[0].module_parameters)
print(system.inverter_parameters)

Expand All @@ -140,16 +146,16 @@ provided for each array, and the arrays are provided to
.. ipython:: python

module_parameters = {'pdc0': 5000, 'gamma_pdc': -0.004}
mount = pvsystem.FixedMount(surface_tilt=20, surface_azimuth=180)
array_one = pvsystem.Array(mount=mount, module_parameters=module_parameters)
array_two = pvsystem.Array(mount=mount, module_parameters=module_parameters)
mount = pvsystem.FixedMount(surface_tilt=20, surface_azimuth=180, racking_model='close_mount')
array_one = pvsystem.Array(mount=mount, module_parameters=module_parameters, module_type='glass_glass')
array_two = pvsystem.Array(mount=mount, module_parameters=module_parameters, module_type='glass_glass')
system_two_arrays = pvsystem.PVSystem(arrays=[array_one, array_two],
inverter_parameters=inverter_parameters)
print([array.module_parameters for array in system_two_arrays.arrays])
print(system_two_arrays.inverter_parameters)


The :py:class:`~pvlib.pvsystem.Array` class includes those
The :py:class:`~pvlib.pvsystem.Array` class includes those
:py:class:`~pvlib.pvsystem.PVSystem` attributes that may vary from array
to array. These attributes include
`module_parameters`, `temperature_model_parameters`, `modules_per_string`,
Expand All @@ -171,7 +177,7 @@ PVSystem attributes
-------------------

Here we review the most commonly used PVSystem and Array attributes.
Please see the :py:class:`~pvlib.pvsystem.PVSystem` and
Please see the :py:class:`~pvlib.pvsystem.PVSystem` and
:py:class:`~pvlib.pvsystem.Array` class documentation for a
comprehensive list of attributes.

Expand All @@ -182,14 +188,27 @@ Tilt and azimuth
The first parameters which describe the DC part of a PV system are the tilt
and azimuth of the modules. In the case of a PV system with a single array,
these parameters can be specified using the `PVSystem.surface_tilt` and
`PVSystem.surface_azimuth` attributes. This will automatically create
`PVSystem.surface_azimuth` attributes.

Temperature model parameters
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The `PVSystem.temperature_model_parameters` attribute must be specified
for further downstream modeling with `ModelChain`.
In lieu of this attribute, the `PVSystem.racking_model` and
`PVSystem.module_type` can be specified, in which case the temperature model
parameters will be inferred.

This will automatically create
an :py:class:`~pvlib.pvsystem.Array` with a :py:class:`~pvlib.pvsystem.FixedMount`
at the specified tilt and azimuth:
at the specified tilt and azimuth and temperature model parameters:

.. ipython:: python

# single south-facing array at 20 deg tilt
system_one_array = pvsystem.PVSystem(surface_tilt=20, surface_azimuth=180)
system_one_array = pvsystem.PVSystem(surface_tilt=20,
surface_azimuth=180,
temperature_model_parameters=temp_params)
print(system_one_array.arrays[0].mount)


Expand All @@ -199,9 +218,11 @@ for each array by passing a different :py:class:`~pvlib.pvsystem.FixedMount`

.. ipython:: python

array_one = pvsystem.Array(pvsystem.FixedMount(surface_tilt=30, surface_azimuth=90))
mount_one = pvsystem.FixedMount(surface_tilt=30, surface_azimuth=90, racking_model='close_mount')
array_one = pvsystem.Array(mount=mount_one, module_type='glass_glass')
print(array_one.mount.surface_tilt, array_one.mount.surface_azimuth)
array_two = pvsystem.Array(pvsystem.FixedMount(surface_tilt=30, surface_azimuth=220))
mount_two = pvsystem.FixedMount(surface_tilt=30, surface_azimuth=220, racking_model='close_mount')
array_two = pvsystem.Array(mount=mount_two, module_type='glass_glass')
system = pvsystem.PVSystem(arrays=[array_one, array_two])
system.num_arrays
for array in system.arrays:
Expand All @@ -220,7 +241,7 @@ and `solar_azimuth` as arguments.
.. ipython:: python

# single south-facing array at 20 deg tilt
system_one_array = pvsystem.PVSystem(surface_tilt=20, surface_azimuth=180)
system_one_array = pvsystem.PVSystem(surface_tilt=20, surface_azimuth=180, temperature_model_parameters=temp_params)
print(system_one_array.arrays[0].mount)

# call get_aoi with solar_zenith, solar_azimuth
Expand All @@ -234,7 +255,9 @@ operates in a similar manner.
.. ipython:: python

# two arrays each at 30 deg tilt with different facing
array_one = pvsystem.Array(pvsystem.FixedMount(surface_tilt=30, surface_azimuth=90))
mount = pvsystem.FixedMount(surface_tilt=30, surface_azimuth=90)
array_one = pvsystem.Array(mount=mount,
temperature_model_parameters=temp_params)
array_one_aoi = array_one.get_aoi(solar_zenith=30, solar_azimuth=180)
print(array_one_aoi)

Expand All @@ -245,7 +268,9 @@ operates on all `Array` instances in the `PVSystem`, whereas the the

.. ipython:: python

array_two = pvsystem.Array(pvsystem.FixedMount(surface_tilt=30, surface_azimuth=220))
mount = pvsystem.FixedMount(surface_tilt=30, surface_azimuth=220)
array_two = pvsystem.Array(mount=mount,
temperature_model_parameters=temp_params)
system_multiarray = pvsystem.PVSystem(arrays=[array_one, array_two])
print(system_multiarray.num_arrays)
# call get_aoi with solar_zenith, solar_azimuth
Expand Down Expand Up @@ -286,7 +311,8 @@ included with pvlib python by using the :py:func:`~pvlib.pvsystem.retrieve_sam`
inverters = pvsystem.retrieve_sam('cecinverter')
inverter_parameters = inverters['ABB__MICRO_0_25_I_OUTD_US_208__208V_']
system_one_array = pvsystem.PVSystem(module_parameters=module_parameters,
inverter_parameters=inverter_parameters)
inverter_parameters=inverter_parameters,
temperature_model_parameters=temp_params)


The module and/or inverter parameters can also be specified manually.
Expand All @@ -307,7 +333,9 @@ arranged into 5 strings of 7 modules each.

.. ipython:: python

system = pvsystem.PVSystem(modules_per_string=7, strings_per_inverter=5)
system = pvsystem.PVSystem(modules_per_string=7,
strings_per_inverter=5,
temperature_model_parameters=temp_params)
# crude numbers from a single module
data = pd.DataFrame({'v_mp': 8, 'v_oc': 10, 'i_mp': 5, 'i_x': 6,
'i_xx': 4, 'i_sc': 7, 'p_mp': 40}, index=[0])
Expand Down
10 changes: 8 additions & 2 deletions docs/sphinx/source/whatsnew/v0.10.4.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,26 @@ Enhancements

Bug fixes
~~~~~~~~~

* Fixes `AttributeError: 'PVSystem' object has no attribute 'racking_model'`
error in `ModelChain` instantiation when the `temperature_model_parameters`
for a `PVSystem` have not been set. Update requires `PVSystem` or `Array`
to have `temperature_model_parameters` explicitly or implicitly by providing
`racking_model` and `module_type`.
Comment on lines +15 to +18
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
error in `ModelChain` instantiation when the `temperature_model_parameters`
for a `PVSystem` have not been set. Update requires `PVSystem` or `Array`
to have `temperature_model_parameters` explicitly or implicitly by providing
`racking_model` and `module_type`.
error in `ModelChain` instantiation when the `temperature_model_parameters`
for a `PVSystem` have not been set. Update requires `PVSystem` or `Array`
to have `temperature_model_parameters` explicitly or implicitly by providing
`racking_model` and `module_type`.


Testing
~~~~~~~


Documentation
~~~~~~~~~~~~~

* Update all examples in :ref:`pvsystem` User's guide page that reference
`pvsystem` to adhere to new required argument.
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
`pvsystem` to adhere to new required argument.
`pvsystem` to adhere to new required argument.


Requirements
~~~~~~~~~~~~


Contributors
~~~~~~~~~~~~
* :ghuser:`matsuobasho`

4 changes: 1 addition & 3 deletions pvlib/modelchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1118,9 +1118,7 @@ def infer_temperature_model(self):
array.temperature_model_parameters for array in self.system.arrays)
params = _common_keys(temperature_model_parameters)
# 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):
if {'a', 'b', 'deltaT'} <= params:
return self.sapm_temp
elif {'u_c', 'u_v'} <= params:
return self.pvsyst_temp
Expand Down
19 changes: 19 additions & 0 deletions pvlib/pvsystem.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -942,6 +942,13 @@ class Array:

name : str, optional
Name of Array instance.

Raises
------
ValueError
If `temperature_model_parameters` is None or if `racking_module`
in the `mount` attribute and `module_type` are either None or cannot be
used to infer the temperature model parameters.
"""

def __init__(self, mount,
Expand Down Expand Up @@ -977,6 +984,18 @@ def __init__(self, mount,
else:
self.temperature_model_parameters = temperature_model_parameters

if self.temperature_model_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
if self.temperature_model_parameters=={}:
if self.temperature_model_parameters == {}:

Copy link
Member

Choose a reason for hiding this comment

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

@matsuobasho I agree with @kandersolar point of view, that we should remove this ValueError. I think we'll want to undo most of the changes in test_pvsystem.py as well. Because I can push to your fork, let me know if you'd like some help. And thanks again for your effort here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It'll be most efficient to delete this entire MR because there are so many changes that will not be used. The proposed change is just deleting two lines and will be easiest to recreate with a new MR.

Let me know if you can kill the entire MR. I'll then create a new MR with just that one change.

In this case, it looks like we do not need a new test or any docstring modifications?

Copy link
Member

Choose a reason for hiding this comment

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

You can close this PR and start a new one.

For the new PR I recommend that you first create a branch in your fork and use that branch for changes. Its not usually a good idea to make changes in your main branch.

First reset your main branch

git reset --hard upstream/ main where upstream is your name for pvlib/pvlib-python

Then create a branch

git checkout -b

Make the edits in the branch, add, commit, push, etc.

raise ValueError("The `temperature_model_parameters` is empty "
"after an attempt to infer it. "
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
"after an attempt to infer it. "
"after an attempt to infer it. "

"Pass either `temperature_model_parameters` to "
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
"Pass either `temperature_model_parameters` to "
"Pass either `temperature_model_parameters` to "

"`Array` or `PVSystem` (if not passing arrays), or "
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
"`Array` or `PVSystem` (if not passing arrays), or "
"`Array` or `PVSystem` (if not passing arrays), or"

"`racking_module` to the `Array` `mount` "
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
"`racking_module` to the `Array` `mount` "
" `racking_module` to the `Array` `mount` "

"object and `module_type` to `Array. For guidance "
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
"object and `module_type` to `Array. For guidance "
"object and `module_type` to `Array. For guidance"

"on allowed values to use for `racking_module` and "
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
"on allowed values to use for `racking_module` and "
" on values allowed for `racking_module` and "

"`module_type`, examine the "
"TEMPERATURE_MODEL_PARAMETERS global variable from "
"the `temperature` module.")

if array_losses_parameters is None:
self.array_losses_parameters = {}
else:
Expand Down
Loading