SunPower / PVMismatch

An explicit Python PV system IV & PV curve trace calculator which can also calculate mismatch.
http://sunpower.github.io/PVMismatch/
BSD 3-Clause "New" or "Revised" License
79 stars 30 forks source link

instantiating pvsys, if pvmods given but not pvconst, then modules pvconst is different from strings and pvsys #131

Open mikofski opened 4 years ago

mikofski commented 4 years ago

https://github.com/SunPower/PVMismatch/blob/3f88910dd718b393234bb10b27c5426439d63a29/pvmismatch/pvmismatch_lib/pvsystem.py#L39-L45

Should we check modules first and then copy pvconst from there? Which takes precedence? No, actually we don't need to because when a string is instantiated, it checks already to see if the modules have defined a pvconst and cascade it down

The trouble actually starts further down here: https://github.com/SunPower/PVMismatch/blob/3f88910dd718b393234bb10b27c5426439d63a29/pvmismatch/pvmismatch_lib/pvsystem.py#L57

This is an issue because on L41-42 we created a new default pvconst and then passed it to the string which ignores it because the pvconst in pvmods tool precedence - whether it should or not's a different issue. But then on L57 we don't use the pvconst from the new string instead we take the default one, and voila 2 different pvconst.

IMO, the code in both pvsystem and pvstring seem to indicate that if a module or string is given, then it must have a pvconst, and that should take precedence. I think we should stick with that paradigm, so in that case, let the system instantiation go all the way to the bottom and then grab the single pvconst used everywhere for each component on the way back up. This would require changes to both pvsystem:

    def __init__(self, pvconst=None, numberStrs=NUMBERSTRS,
                 pvstrs=None, numberMods=NUMBERMODS, pvmods=None):
        # is pvstrs a list?
        try:
            pvstr0 = pvstrs[0]
        except TypeError:
            # is pvstrs a PVstring object?
            try:
                pvconst = pvstrs.pvconst
            except AttributeError:
                # create a pvstring
                pvstrs = PVstring(numberMods=numberMods, pvmods=pvmods,
                                  pvconst=pvconst)
                # pvconstants from the string takes precedence
                pvconst = pvstrs.pvconst
            # expand pvstrs to list
            pvstrs = [pvstrs] * numberStrs
            numberMods = [numberMods] * numberStrs
        else:
            pvconst = pvstr0.pvconst
            numberStrs = len(pvstrs)
            numberMods = []
            for p in pvstrs:
                if p.pvconst is not pvconst:
                    raise Exception('pvconst must be the same for all strings')
                numberMods.append(len(p.pvmods))
        self.pvconst = pvconst  #: ``PVconstants`` used in ``PVsystem``

and pvstring:

class PVstring(object):
    """
    A class for PV strings.

    :param numberMods: number of modules in string
    :param pvmods: either a list of modules, an instance of
        :class:`~pvmismatch.pvmismatch_lib.pvmodule.PVmodule` [Optional]
    :type pvmods: list, :class:`~pvmismatch.pvmismatch_lib.pvmodule.PVmodule`
    :param pvconst: a configuration constants object
    """
    def __init__(self, numberMods=NUMBERMODS, pvmods=None,
                 pvconst=None):
        # is pvmods a list?
        try:
            pvmod0 = pvmods[0]
        except TypeError:
            # is pvmods an object?
            try:
                pvconst = pvmods.pvconst
            except AttributeError:
                # create pvmod
                pvmods = PVmodule(pvconst=pvconst)
                # pvconstants from the module takes precedence
                pvconst = pvmods.pvconst
            # expand pvmods to list
            pvmods = [pvmods] * numberMods
        else:
            pvconst = pvmod0.pvconst
            numberMods = len(pvmods)
            for p in pvmods:
                if p.pvconst is not pvconst:
                    raise Exception('pvconst must be the same for all modules')
        self.pvconst = pvconst  #: ``PVconstants`` used in  ``PVstring``

related issues:

not related: