-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix error message in ModelChain pertaining to temp_model inference #1946
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
Comments
I started to tackle this issue. 3 questions:
I did some experimentation when specifying 2 arrays for the location = Location(latitude=32.2, longitude=-110.9)
modules = pvsystem.retrieve_sam('cecmod')
module_parameters = modules['Canadian_Solar_Inc__CS5P_220M']
inverter_parameters = {'pdc0': 5000, 'eta_inv_nom': 0.96}
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, temperature_model_parameters={'a': -3.47, 'b': -.0594, 'deltaT': 3})
array_two = pvsystem.Array(mount=mount, module_parameters=module_parameters, temperature_model_parameters={'a': -3.56, 'b': -.0750, 'deltaT': 3}
system_two_arrays = pvsystem.PVSystem(arrays=[array_one, array_two],
inverter_parameters=inverter_parameters)
modelchain = ModelChain(
system_two_arrays, location, name='dummy_name', aoi_model="physical", temperature_model="sapm", spectral_model="no_loss"
) I'm not following the reasoning of the last sentence in the error message. If we specify any dictionary from the same temp model family as above, then we don't get the error at all.
However, if we specify one array to be from 'sapm' and the other one from 'pvsyst' then the error reads:
|
@cwhanse or one of the other maintainers, can you please provide some pointers on the above. I would like to start working to resolve this issue. Thanks. |
@matsuobasho pvlib's ModelChain currently only supports a single temperature model for all arrays. There's no way to set a temperature model for each array. We don't need to test or provide an error message if a user specifies temperature model parameters from different models, for different arrays. We should allow for a user to specify different parameters for the same temperature model, as is done in your example with SAPM. When I run that code, I don't get the error, even after adding a modelchain.run_model call. I think that is correct behavior. Re-reading that error message text, I would edit it as: "Could not infer the temperature model. If Arrays are used to construct the PVSystem, check that all Arrays in ModelChain.system.arrays have parameters for the same temperature model. If Arrays are not used, check that the PVsystem attributes racking_model and module_type are valid." I'm now on the fence if returning " Common temperature model parameters: {params}." is useful. |
Regarding your first point, the following comment really pertains to #1942 - the assumption is that the user understands that different parameters in the format I'll incorporate the wording you propose. Can you please make a recommendation about whether the wording of the other messages in |
Let's address only the message for the temperature model this issue. When we get concurrence on that change, we can make similar changes to the other messages. |
I submitted the PR 1 week ago, requesting feedback on it. Thank you. |
@matsuobasho can you fix the linter issues? I could push to your fork if you would like help. |
Thanks @cwhanse should have seen that fail in the build. Pushed the update now, please let me know if there's anything else. |
Describe the bug
Error message is incorrect.
The message is:
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().
However, in code below we are creating a PVSystem without using
Arrays
. So the second sentence in the message above is not describing the proper way to fix this issue.To Reproduce
Expected behavior
Error message correctly explains what needs to be changed in
pvsystem
insantiation in order forModelChain
to run.Versions:
pvlib.__version__
: 0.10.2Additional context
This error message also appears in other contexts. Namely for other model types:
AC
AOI
DC
spectral
So the error messages for those models should be fixed as well. Search for 'could not infer' to see which messages.
The text was updated successfully, but these errors were encountered: