BUG: pvconst is not a singleton, inconsistent, doesn't update (GH38) (#…

…62)

* fixes #59

* add class attr _calc_now
* override _calc_now with instance attribte at the end of constructor
* check if _calc_now is True to decide to call self.calcCell() instead
  of checking for pvconst
* use self.__dict__.update() instead of using
  super(PVcell, self).__setattr__(key, value)
* in update, remove TODO, b/c even tho __dict__.update() would bypass
  __setattr__() then would have to check for floats, and still set
  _calc_now = True to trigger recalc, so instead, just set
  _calc_now = False first to turn calculations off, until all attr are
  set, then recalc

* don't set pvcell.pvconst in pvstring

* just raise an exception if they don't match for now
* remove comments about "deepcopy" everywhere

* oops, recalculate means _calc_now = True, duh!

* remove commented legacy code in __setattr__, add comment in update re checking for floats

* add test for new _calc_now flag

* if _calc_now == False, then calcCell() is not called in __setattr__
* if _calc_now == True, then calcCell() is called in __setattr__

* closes #38 consistent pvconst behavior

* use duck typing to check pvstrs in PVsystem() for list, object or none
* set pvconst from pvstrs if given
* set numbstrs from pvstrs if given
* set numbermods form pvstrs.pvmods if given
* check that pvconst and pvmods are consistent
* add tests

* apply same changes in pvsystem from last commit to pvstr

* change default pvconst arg to None
* use duck typing to check if pvmods is a list, an object, or None
* relax requirement that all strings have same number of modules
* change pvsys.numberMods to a list of number of modules in each string
* add test to check pvstring
* check that all pvconst are the same for all modules
* add docstring for members

* also test that pvsys.numberMods is now a list

* each item in list is number of modules in corresponding string

* use ducktyping to determine if pvcells is list or obj or None

* add missing blank lines and wrap long lines per pep8
* add pvcell object to pvcells docstring arg type
* add tests in test_module to check that pvconst is the same for module
  and all cells

* add test for update method

* replace npts attr with a property

* add new private _npts attribute, return for npts in getter
* set npts, pts, negpts, Imod_pts, and Imod_negpts in setter
* add test to show that changing npts changes pts, negpts, Imod_pts, and
  Imod_negpts

* add pvsystem update method

* make system calculation DRY, use everywhere calcSystem and
  calcMPP_IscVocFFeff are called back to back to set Isys, Vsys, Psys,
  etc.
* also makes it explicity to recalc the system after a change, like
  change pvconst.npts

pvmismatch/pvmismatch_lib/pvconstants.py

@@ -74,23 +74,39 @@ class PVconstants(object):
     T0 = 298.15  #: [K] reference temperature
 
     def __init__(self, npts=NPTS):
-        # set number of points in IV curve(s)
-        self.npts = npts  #: number of points in IV curves
-        # point spacing from 0 to 1, used for Vcell, Vmod, Vsys and Istring
-        # decrease point spacing as voltage approaches Voc by using logspace
-        pts = (11. - np.logspace(np.log10(11.), 0., self.npts)) / 10.
-        pts[0] = 0.  # first point must be exactly zero
-        self.pts = pts.reshape((self.npts, 1))
+        self._npts = None
+        self.pts = None
         """array of points with decreasing spacing from 0 to 1"""
-        negpts = (11. - np.logspace(np.log10(11. - 1. / float(self.npts)),
-                                    0., self.npts)) / 10.
-        negpts = negpts.reshape((self.npts, 1))
-        self.Imod_negpts = 1 + 1. / float(self.npts) / 10. - negpts
+        self.Imod_negpts = None
         """array of points with decreasing spacing from 1 to just less than but
         not including zero"""
-        self.negpts = np.flipud(negpts)  # reverse the order
+        self.negpts = None
         """array of points with increasing spacing from 1 to just less than but
         not including zero"""
+        self.Imod_pts = None
+        """array of points with increasing spacing from 0 to 1"""
+        # call property setter
+        self.npts = npts  #: number of points in IV curves
+
+    @property
+    def npts(self):
+        """number of points in IV curves"""
+        return self._npts
+
+    @npts.setter
+    def npts(self, npts):
+        # set number of points in IV curve(s)
+        self._npts = npts  # number of points in IV curves
+        # point spacing from 0 to 1, used for Vcell, Vmod, Vsys and Istring
+        # decrease point spacing as voltage approaches Voc by using logspace
+        pts = (11. - np.logspace(np.log10(11.), 0., self._npts)) / 10.
+        pts[0] = 0.  # first point must be exactly zero
+        self.pts = pts.reshape((self._npts, 1))
+        negpts = (11. - np.logspace(np.log10(11. - 1. / float(self._npts)),
+                                    0., self._npts)) / 10.
+        negpts = negpts.reshape((self._npts, 1))
+        self.Imod_negpts = 1 + 1. / float(self._npts) / 10. - negpts
+        self.negpts = np.flipud(negpts)  # reverse the order
         # shift and concatenate pvconst.negpts and pvconst.pts
         # so that tight spacing is around MPP and RBD
         self.Imod_pts = 1 - np.flipud(self.pts)

pvmismatch/pvmismatch_lib/pvmodule.py

@@ -57,6 +57,7 @@ def standard_cellpos_pat(nrows, ncols_per_substr):
         cellpos.append(newsubstr)
     return cellpos
 
+
 # standard cell positions presets
 STD24 = standard_cellpos_pat(1, [1] * 24)
 STD72 = standard_cellpos_pat(12, [2, 2, 2])
@@ -122,6 +123,7 @@ def crosstied_cellpos_pat(nrows_per_substrs, ncols, partial=False):
 Substrings have 27, 28 and 27 rows of cells per diode
 """
 
+
 def combine_parallel_circuits(IVprev_cols, pvconst):
     """
     Combine crosstied circuits in a substring
@@ -151,33 +153,50 @@ def combine_parallel_circuits(IVprev_cols, pvconst):
         Irows, Vrows, Isc_rows.mean(), Imax_rows.max()
     )
 
+
 class PVmodule(object):
     """
     A Class for PV modules.
 
     :param cell_pos: cell position pattern
     :type cell_pos: dict
     :param pvcells: list of :class:`~pvmismatch.pvmismatch_lib.pvcell.PVcell`
-    :type pvcells: list
+    :type pvcells: list, :class:`~pvmismatch.pvmismatch_lib.pvcell.PVcell`
     :param pvconst: An object with common parameters and constants.
     :type pvconst: :class:`~pvmismatch.pvmismatch_lib.pvconstants.PVconstants`
     :param Vbypass: bypass diode trigger voltage [V]
     :param cellArea: cell area [cm^2]
     """
-    def __init__(self, cell_pos=STD96, pvcells=None, pvconst=PVconstants(),
+    def __init__(self, cell_pos=STD96, pvcells=None, pvconst=None,
                  Vbypass=VBYPASS, cellArea=CELLAREA):
         # TODO: check cell position pattern
         self.cell_pos = cell_pos  #: cell position pattern dictionary
         self.numberCells = sum([len(c) for s in self.cell_pos for c in s])
         """number of cells in the module"""
+        # is pvcells a list?
+        try:
+            pvc0 = pvcells[0]
+        except TypeError:
+            # is pvcells an object?
+            try:
+                pvconst = pvcells.pvconst
+            except AttributeError:
+                #  try to use the pvconst arg or create one if none
+                if not pvconst:
+                    pvconst = PVconstants()
+                # create pvcell
+                pvcells = PVcell(pvconst=pvconst)
+            # expand pvcells to list
+            pvcells = [pvcells] * self.numberCells
+        else:
+            pvconst = pvc0.pvconst
+            for p in pvcells:
+                if p.pvconst is not pvconst:
+                    raise Exception('PVconstant must be the same for all cells')
         self.pvconst = pvconst  #: configuration constants
         self.Vbypass = Vbypass  #: [V] trigger voltage of bypass diode
         self.cellArea = cellArea  #: [cm^2] cell area
-        if pvcells is None:
-            pvcells = PVcell(pvconst=self.pvconst)
-        # expand pvcells to list
-        if isinstance(pvcells, PVcell):
-            pvcells = [pvcells] * self.numberCells
+        # check cell position pattern matches list of cells
         if len(pvcells) != self.numberCells:
             # TODO: use pvexception
             raise Exception(
@@ -291,8 +310,10 @@ def setSuns(self, Ee, cells=None):
                 raise Exception("Input irradiance value (Ee) for each cell!")
         self.Imod, self.Vmod, self.Pmod, self.Isubstr, self.Vsubstr = self.calcMod()
 
-# TODO setTemps is a nearly identical copy of setSuns. The DRY principle says that we should not be copying code.
-# TODO Replace both setSuns() and setTemps() with a single method for updating cell parameters that works for all params
+    # TODO setTemps is a nearly identical copy of setSuns. The DRY principle
+    # says that we should not be copying code.
+    # TODO Replace both setSuns() and setTemps() with a single method for
+    # updating cell parameters that works for all params
 
     def setTemps(self, Tc, cells=None):
         """

pvmismatch/pvmismatch_lib/pvstring.py

@@ -26,26 +26,32 @@ class PVstring(object):
     :param pvconst: a configuration constants object
     """
     def __init__(self, numberMods=NUMBERMODS, pvmods=None,
-                 pvconst=PVconstants()):
-        self.pvconst = pvconst
-        self.numberMods = numberMods
-        if pvmods is None:
-            pvmods = PVmodule(pvconst=self.pvconst)
-        # expand pvmods to list
-        if isinstance(pvmods, PVmodule):
-            pvmods = [pvmods] * self.numberMods
-        if len(pvmods) != self.numberMods:
-            # TODO: use pvmismatch exceptions
-            raise Exception("Number of modules doesn't match.")
-        # check that pvconst if given, is the same for all cells
-        # don't assign pvcell.pvconst here since it triggers a recalc
-        for p in pvmods:
-            for c in p.pvcells:
-                if c.pvconst is not self.pvconst:
-                    raise Exception('PVconstant must be the same for all cells')
-            if p.pvconst is not self.pvconst:
-                raise Exception('PVconstant must be the same for all cells')
-        self.pvmods = pvmods
+                 pvconst=None):
+        # is pvmods a list?
+        try:
+            pvmod0 = pvmods[0]
+        except TypeError:
+            # is pvmods an object?
+            try:
+                pvconst = pvmods.pvcons
+            except AttributeError:
+                #  try to use the pvconst arg or create one if none
+                if not pvconst:
+                    pvconst = PVconstants()
+                # create pvmod
+                pvmods = PVmodule(pvconst=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``
+        self.numberMods = numberMods  #: number of module in string
+        self.pvmods = pvmods  #: list of ``PVModule`` in ``PVstring``
+        # calculate string
         self.Istring, self.Vstring, self.Pstring = self.calcString()
 
     # TODO: use __getattr__ to check for updates to pvcells

pvmismatch/pvmismatch_lib/pvsystem.py

@@ -27,21 +27,42 @@ class PVsystem(object):
     :param numberMods: number of modules per string
     :param pvmods: list of modules, a ``PVmodule`` object or None
     """
-    def __init__(self, pvconst=PVconstants(), numberStrs=NUMBERSTRS,
+    def __init__(self, pvconst=None, numberStrs=NUMBERSTRS,
                  pvstrs=None, numberMods=NUMBERMODS, pvmods=None):
-        self.pvconst = pvconst
-        self.numberStrs = numberStrs
-        self.numberMods = numberMods
-        if pvstrs is None:
-            pvstrs = PVstring(numberMods=self.numberMods, pvmods=pvmods,
-                              pvconst=self.pvconst)
-        # expand pvstrs to list
-        if isinstance(pvstrs, PVstring):
-            pvstrs = [pvstrs] * self.numberStrs
-        if len(pvstrs) != self.numberStrs:
-            # TODO: use pvmismatch excecptions
-            raise Exception("Number of strings don't match.")
-        self.pvstrs = pvstrs
+        # is pvstrs a list?
+        try:
+            pvstr0 = pvstrs[0]
+        except TypeError:
+            # is pvstrs a PVstring object?
+            try:
+                pvconst = pvstrs.pvconst
+            except AttributeError:
+                # try to use the pvconst arg or create one if none
+                if not pvconst:
+                    pvconst = PVconstants()
+                # create a pvstring
+                pvstrs = PVstring(numberMods=numberMods, pvmods=pvmods,
+                                  pvconst=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``
+        self.numberStrs = numberStrs  #: number strings in the system
+        self.numberMods = numberMods  #: list of number of modules per string
+        self.pvstrs = pvstrs  #: list of ``PVstring`` in system
+        # calculate pvsystem
+        self.update()
+
+    def update(self):
+        """Update system calculations."""
         self.Isys, self.Vsys, self.Psys = self.calcSystem()
         (self.Imp, self.Vmp, self.Pmp,
          self.Isc, self.Voc, self.FF, self.eff) = self.calcMPP_IscVocFFeff()
@@ -124,9 +145,8 @@ def setSuns(self, Ee):
                 pvstr = int(pvstr)
                 self.pvstrs[pvstr] = copy(self.pvstrs[pvstr])
                 self.pvstrs[pvstr].setSuns(pvmod_Ee)
-        self.Isys, self.Vsys, self.Psys = self.calcSystem()
-        (self.Imp, self.Vmp, self.Pmp,
-         self.Isc, self.Voc, self.FF, self.eff) = self.calcMPP_IscVocFFeff()
+        # calculate pvsystem
+        self.update()
 
     def setTemps(self, Tc):
         """
@@ -162,9 +182,8 @@ def setTemps(self, Tc):
                 pvstr = int(pvstr)
                 self.pvstrs[pvstr] = copy(self.pvstrs[pvstr])
                 self.pvstrs[pvstr].setTemps(pvmod_Tc)
-        self.Isys, self.Vsys, self.Psys = self.calcSystem()
-        (self.Imp, self.Vmp, self.Pmp,
-         self.Isc, self.Voc, self.FF, self.eff) = self.calcMPP_IscVocFFeff()
+        # calculate pvsystem
+        self.update()
 
     def plotSys(self, sysPlot=None):
         """

pvmismatch/tests/test_pvcell.py

@@ -71,6 +71,9 @@ def test_pvcell_calc_rbd():
 
 
 def test_pvcell_calc_now_flag():
+    """
+    Test ``_calc_now`` turns off recalc in ``__setattr__``.
+    """
     pvc = PVcell()
     itest, vtest, ptest = pvc.Icell, pvc.Vcell, pvc.Pcell
     pvc._calc_now = False
@@ -85,5 +88,18 @@ def test_pvcell_calc_now_flag():
     assert np.allclose(pcell, pvc.Pcell)
 
 
+def test_update():
+    pvc = PVcell()
+    Rs = pvc.Rs
+    itest = pvc.Icell[170]
+    pvc.update(Rs=0.001)
+    assert np.isclose(pvc.Icell[170], 5.79691674)
+    pvc._calc_now = False
+    pvc.Rs = Rs
+    pvc.update()  # resets _calc_now to True
+    assert np.isclose(pvc.Icell[170], itest)
+    assert pvc._calc_now
+
+
 if __name__ == "__main__":
     test_calc_series()

pvmismatch/tests/test_pvconstants.py

@@ -0,0 +1,21 @@
+from pvmismatch import *
+
+
+def test_pvconst_npts_setter():
+    """Test pvconst property and setter methods"""
+    pvconst = pvconstants.PVconstants()
+    assert pvconst.npts == pvconstants.NPTS
+    assert len(pvconst.pts) == pvconst.npts
+    assert pvconst.pts[0] == 0
+    assert pvconst.pts[-1] == 1
+    assert len(pvconst.negpts) == pvconst.npts
+    assert pvconst.negpts[0] == 1
+    assert pvconst.negpts[-1] > 0
+    pvconst.npts = 1001
+    assert pvconst.npts == 1001
+    assert len(pvconst.pts) == pvconst.npts
+    assert pvconst.pts[0] == 0
+    assert pvconst.pts[-1] == 1
+    assert len(pvconst.negpts) == pvconst.npts
+    assert pvconst.negpts[0] == 1
+    assert pvconst.negpts[-1] > 0

pvmismatch/tests/test_pvmodule.py

@@ -4,6 +4,7 @@
 
 from nose.tools import ok_
 from pvmismatch.pvmismatch_lib.pvmodule import PVmodule, TCT492, PCT492
+from pvmismatch.pvmismatch_lib.pvcell import PVcell
 import numpy as np
 from copy import copy
 
@@ -40,6 +41,30 @@ def test_calc_pct_bridges():
     pvmod = PVmodule(cell_pos=pct492_bridges)
     return pvmod
 
+
+def check_same_pvconst_and_lengths(pvmod):
+    assert len(pvmod.pvcells) == 96
+    for p in pvmod.pvcells:
+        assert p.pvconst is pvmod.pvconst
+
+
+def test_pvmodule_with_pvcells_list():
+    pvcells = [PVcell()] * 96
+    pvmod = PVmodule(pvcells=pvcells)
+    check_same_pvconst_and_lengths(pvmod)
+
+
+def test_pvmodule_with_pvcells_obj():
+    pvcells = PVcell()
+    pvmod = PVmodule(pvcells=pvcells)
+    check_same_pvconst_and_lengths(pvmod)
+
+
+def test_pvmodule_with_no_pvcells():
+    pvmod = PVmodule()
+    check_same_pvconst_and_lengths(pvmod)
+
+
 if __name__ == "__main__":
     test_calc_mod()
     test_calc_tct_mod()

pvmismatch/tests/test_pvstring.py

@@ -0,0 +1,24 @@
+from pvmismatch import *
+
+
+def check_same_pvconst_and_lengths(pvstr):
+    assert len(pvstr.pvmods) == pvstring.NUMBERMODS
+    for p in pvstr.pvmods:
+        assert p.pvconst is pvstr.pvconst
+
+
+def test_pvstring_with_pvmods_list():
+    pvmods = [pvmodule.PVmodule()] * pvstring.NUMBERMODS
+    pvstr = pvstring.PVstring(pvmods=pvmods)
+    check_same_pvconst_and_lengths(pvstr)
+
+
+def test_pvstring_with_pvmods_obj():
+    pvmods = pvmodule.PVmodule()
+    pvstr = pvstring.PVstring(pvmods=pvmods)
+    check_same_pvconst_and_lengths(pvstr)
+
+
+def test_pvstring_with_no_pvmods():
+    pvstr = pvstring.PVstring()
+    check_same_pvconst_and_lengths(pvstr)