Make interp IAM method available for modelchain

[echedey-ls]

• Closes Add interp method for modelchain aoi model. #1742
• I am familiar with the contributing guidelines
• Tests added
• [ ] Updates entries in docs/sphinx/source/reference for API changes.
• Adds description and name entries in the appropriate "what's new" file in docs/sphinx/source/whatsnew for all changes. Includes link to the GitHub Issue with :issue:`num` or this Pull Request with :pull:`num`. Includes contributor name and/or GitHub username (link with :ghuser:`user`).
• New code is fully documented. Includes numpydoc compliant docstrings, examples, and comments where necessary.
• Pull request is nearly complete and ready for detailed review.
• Maintainer: Appropriate GitHub Labels (including remote-data) and Milestone are assigned to the Pull Request and linked Issue.

Have a look at #1742

I have some doubts about tests, there are many of them testing aoi losses models, but 'interp' requires some arguments (it does not have defaults to run without args like the other ones in test_aoi_models among other tests). I ended up creating a new one.
In this specific test, I've modified the fixture to add the required args iam_ref and theta_ref to the module in the PVSystem.

[kandersolar]

Thanks @echedey-ls I haven't tried it out yet but this seems great on a first glance! A couple initial comments below.

[kandersolar]

This is looking good to me. A couple small changes and I think this will be good to merge!

[kandersolar]

Makes me wonder if the IAM parameters should have been tracked in their own separate data structure instead of having to be extracted from module_parameters. (no action needed in this PR; just thinking out loud)

[cwhanse]

I think they only reason they were in module_parameters is because the old SAPM database includes them. That's not a reason to keep this as they are, in my view.

[kandersolar]

I notice also that we don't have a way to pass upper through to pvlib.iam.sapm. Separate issue.

[echedey-ls]

Hmmmm. I don't like many things about the current interface of these IAM functions; IMO, a good approach is shown at physical:

def physical(aoi, n=1.526, K=4.0, L=0.002, *, n_ar=None):

Note the asterisk (it means following args are keyword-only args just in case).

Each argument, of different kinds, parse to a real-world meaning (again, IMO).

POSITIONAL_ONLY (before /) & POSITIONAL_OR_KEYWORD: used for required input to run models. In the case of AOI modifiers, it is the aoi parameter. We see that they can also be used to specify defaults for the model params. {aoi, n, K, L}

KEYWORD_ONLY: used to specify additional inputs, that are not required in the published paper of the model. {n_ar}

I would still upgrade the function signature to:

def physical(aoi, /, n=1.526, K=4.0, L=0.002, *, n_ar=None):

So it is always the first parameter the input. This is completely unnecessary in IAM models since the only required input to work is aoi.

Why this proposal?

Because of the inspect module. In this PR I used it to retrieve all the parameters needed:

if model in ['ashrae', 'physical', 'martin_ruiz', 'interp']:
    func = getattr(iam, model)  # get function at pvlib.iam
    # get all parameters from function signature to retrieve them from
    # module_parameters if present
    params = set(inspect.signature(func).parameters.keys())
    params.discard('aoi')  # exclude aoi so it can't be repeated
    kwargs = _build_kwargs(params, self.module_parameters)
    return func(aoi, **kwargs)

With this broad approach to the function parameter meanings, we could test for the types of each one and infer it's use with inspect: we could stop maintaining iam._IAM_MODEL_PARAMETERS, which only is used to infer the right model as stated by @cwhanse . Except for iam.sapm, its interface is special but could be changed easily.

I tried not to use iam._IAM_MODEL_PARAMETERS in the whole code, but with no luck OFC, functions interfaces had to be changed - a breaking API change.

In short,

I propose a normalized way of defining function signatures, where:

• POSITIONAL_ONLY arguments are the required ones to get numerical values that may make sense (this is open to debate [1])
• POSITIONAL_OR_KEYWORD are the ones that fit a function or a physical magnitude of interest, that usually (always?) has a default values
• KEYWORD_ONLY args are completely optional to run the model
  So with the inspect module utilities all this information is easily obtained and no more inference dicts are needed.

[1] Since also required ones are those without default values and it can also be checked with the inspect module.

In the case of IAM module:

def ashrae(aoi, /, b=0.05):
def physical(aoi, /, n=1.526, K=4.0, L=0.002, *, n_ar=None):
def martin_ruiz(aoi, /, a_r=0.16):
def martin_ruiz_diffuse(surface_tilt, /, a_r=0.16, c1=0.4244, *, c2=None):
def interp(aoi, theta_ref, iam_ref, *, method='linear', normalize=True):
def sapm(aoi, /, module, *, upper=None):  # here module should be changed to B5 ... B0
...

Or without the /,, as stated per [1].

I see this got really long, may I open a discussion?

[kandersolar]

may I open a discussion?

Absolutely! Better to have a dedicated thread for this. Otherwise it will get buried soon when the PR is merged :)

[kandersolar]

Thanks again @echedey-ls! This is a nice contribution.

[toddkarin]

Thank you!

I wrote a minimal example file to show how to use it (please let me know if I did anything wrong).

import pandas as pd
import numpy as np
import pvlib
from pvlib.pvsystem import PVSystem, FixedMount
from pvlib.location import Location
from pvlib.modelchain import ModelChain
from pvlib.temperature import TEMPERATURE_MODEL_PARAMETERS

temperature_model_parameters = TEMPERATURE_MODEL_PARAMETERS['sapm']['open_rack_glass_glass']


sandia_modules = pvlib.pvsystem.retrieve_sam('SandiaMod')
cec_inverters = pvlib.pvsystem.retrieve_sam('cecinverter')

sandia_module = sandia_modules['Canadian_Solar_CS5P_220M___2009_']

# Specify the IAM curve
sandia_module['theta_ref'] = [0, 30, 50,60, 70, 75,80, 85, 90]
sandia_module['iam_ref'] = [1.000, 0.999, 0.987, 0.962, 0.892, 0.816, 0.681, 0.440, 0.0]
# sandia_module['iam_ref'] = [1.000, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]

print('sandia_module', sandia_module)

cec_inverter = cec_inverters['ABB__MICRO_0_25_I_OUTD_US_208__208V_']

location = Location(latitude=32.2, longitude=-110.9)
system = PVSystem(surface_tilt=20, surface_azimuth=200,
                  module_parameters=sandia_module,
                  inverter_parameters=cec_inverter,
                  temperature_model_parameters=temperature_model_parameters)


mc = ModelChain(system, location)

weather = pd.DataFrame([[1050, 1000, 100, 30, 5]],
                       columns=['ghi', 'dni', 'dhi', 'temp_air', 'wind_speed'],
                       index=[pd.Timestamp('20170401 1200', tz='US/Arizona')])

mc.aoi_model = 'interp'

mc.run_model(weather)

print(mc.results.aoi_modifier)

[toddkarin]

@cwhanse, @echedey-ls , @kandersolar

Is this technique implemented for single axis trackers? If someone can provide a minimal example using SAT that would be great!

[cwhanse]

@toddkarin ModelChain doesn't care too much about its PVSystem. Define a system with an Array of type SingleAxisTrackerMount similar to this example, assign mc.aoi_model = 'interp' as above or do it at instantiation mc = ModelChain(..., aoi_model='interp', ...)

[kandersolar]

@toddkarin want to submit a PR to add that script to the example gallery?

[echedey-ls]

That would be amazing!