Vectorize geometry calculations #60

anomam · 2019-07-25T09:37:10Z

Vectorize geometry calculations of ordered PV arrays to improve computation time

In OrderedPVArray:

PV row coordinates calculation can be vectorized
Ground shadows and cut points coordinates calculation can also be vectorized
This means that the class will be instantiated and used differently.
Update OrderedPVArray class to use a fit()/transform() approach, just like the irradiance models
Update PVEngine to accept that change from PV arrays from now on
Update run function as necessary

* next is calculating shadows...!

* fixed tests accordingly

* because pvrows and ground can change, so it's hard to know if an update needs to be done or not

* the edge pts list was not re-initialized at every interation

* shaded length is somehow infinitesimally smaller than with previous class, but this has virtually no impact. To be investigated later * now surfaces have more logical indexing (from left to right, increasing index) in ground object, so matrices ordering got shifted a bit

anomam · 2019-07-30T08:47:33Z

pvfactors/engine.py

@@ -14,9 +13,8 @@ class PVEngine(object):
    as a timeseries when the pvarrays can be build from dictionary parameters
    """

-    def __init__(self, params, vf_calculator=VFCalculator(),
+    def __init__(self, pvarray, vf_calculator=VFCalculator(),


it's meaningless to have pvarray=OrderedPVArray(), because it will make something fail later.
It's better to initialize the PV array outside of the engine and then pass it in

anomam · 2019-07-30T08:48:21Z

pvfactors/engine.py

        self.vf_calculator = vf_calculator
        self.irradiance = irradiance_model
-        self.cls_pvarray = cls_pvarray
-        self.is_fast_mode = isinstance(fast_mode_pvrow_index, int) \
-            and fast_mode_pvrow_index < params['n_pvrows']


since params is not passed anymore to the engine, we can't do this check for the user anymore

anomam · 2019-07-30T08:49:48Z

pvfactors/engine.py

@@ -108,9 +104,11 @@ def fit(self, timestamps, DNI, DHI, solar_zenith, solar_azimuth,

        # Fit irradiance model
        self.irradiance.fit(timestamps, DNI, DHI, solar_zenith, solar_azimuth,
-                            surface_tilt, surface_azimuth,
-                            self.params['rho_front_pvrow'],
-                            self.params['rho_back_pvrow'], albedo)


now the rho reflectivity values are passed to the irradiance models at initialization: it kind of makes more sense since they are not timeseries values, and bc the fit() function is more or less for timeseries vectorized calculations

anomam · 2019-07-30T08:50:34Z

pvfactors/engine.py

+            pvarray = self.pvarray
+
+            # Transform pvarray to time step
+            pvarray.transform(idx)


now pvarray is also using the "fit()/transform()" paradigm, just like the irradiance models

anomam · 2019-07-30T08:51:52Z

pvfactors/engine.py

-                for idx, surface in geom_dict.items():
-                    surface.update_params({'q0': q0[idx], 'qinc': qinc[idx],
-                                           'isotropic': isotropic_vec[idx],
-                                           'reflection': reflection_vec[idx]})


there was a non-impactful conflict between the idx passed to run_timestep() and the index of this for loop

anomam · 2019-07-30T08:54:51Z

pvfactors/geometry/base.py

    x_center, y_center = xy_center
    radius = length / 2.
+    # Get rotation
+    rotation = get_rotation_from_tilt_azimuth(surface_azimuth, axis_azimuth,
+                                              tilt)


the fn get_rotation_from_tilt_azimuth will be re-used in OrderedPVArray.
The rotation output here could actually be returned by this coords_from_center_tilt_length() fn, but it would not really make sense...

anomam · 2019-07-30T08:57:17Z

pvfactors/geometry/base.py

-        self._surface_registry = None
-        self._view_matrix = None
-        self._obstr_matrix = None
-        self._surfaces_indexed = False


these were used to cache the values of the class properties. But with the new OrderedPVArray, the pv array attributes can change without notice and it doesn't make sense anymore to cache these values: so they are now recalculated every time the properties are called.
(so we try to call the properties only once, or save their values outside if need to be reused)

anomam · 2019-07-30T08:59:01Z

pvfactors/irradiance/models.py

-        self.rho_front = None
-        self.rho_back = None
+        self.rho_front = rho_front
+        self.rho_back = rho_back


now reflectivity values are passed to the irradiance model at initialization, not in the fit() function; which makes more sense since they are always floats

anomam · 2019-07-30T08:59:51Z

pvfactors/run.py

    vf_calculator = cls_vf()
-    eng = cls_engine(pvarray_parameters, cls_pvarray=cls_pvarray,
-                     irradiance_model=irradiance_model,
+    eng = cls_engine(pvarray, irradiance_model=irradiance_model,


now the pvarray needs to be initialized outside of the engine

anomam · 2019-07-30T09:02:47Z

pvfactors/tests/test_geometry/test_data/__init__.py

+     [8, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10, 0, 0, 0, 0, 4],
+     [0, 0, 6, 6, 6, 6, 6, 6, 6, 6, 0, 0, 0, 0, 10, 0, 5],
+     [8, 8, 8, 8, 0, 0, 0, 8, 0, 0, 0, 0, 0, 10, 0, 0, 4],
+     [0, 0, 0, 0, 6, 6, 6, 0, 6, 6, 0, 0, 0, 0, 0, 0, 5],
     [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]],


the same surfaces are created, but their indexing differs now with the new implementation.
So the matrices are slightly shifted

anomam · 2019-07-30T09:04:46Z

pvfactors/tests/test_viewfactors/test_data/__init__.py

+vf_left_cut_sum_axis_one_rounded = np.array(
+    [0.009, 0.346, 0.359, 0.28, 0.268, 0.04, 0.001, 0.372, 0.387,
+     0.346, 0.035, 0.046, 0.064, 0.966, 0.048, 0.976, 0.981, 0.026,
+     0.977, 0.])


I checked that the values of this vector stayed consistent when I updated the matrix above (which was updated because of the new surface indexing of the new class implementation)

anomam · 2019-07-31T09:21:55Z

Hey @cedricleroy , I would really appreciate if you could take a quick look.
There's a lot going on so you don't need to spend too much time on this and on the details. Any feedback would be greatly appreciated!
I will update the docs tutorials in a follow up PR

cedricleroy

Looks good @anomam! Just minor cosmetic comments.

cedricleroy · 2019-07-31T19:46:54Z

pvfactors/engine.py

+            - solar zenith > 90, ie the sun is down
+            - DNI or DHI is negative, which does not make sense
+            - DNI and DHI are both zero
+


A little below in the Returns: It looks like None can be returned as well.

cedricleroy · 2019-07-31T19:51:49Z

pvfactors/geometry/base.py

-    """Calculate ``shapely`` :py:class:`LineString` coordinate from
-    center coords, surface tilt angle and length of line.
+    """Calculate ``shapely`` :py:class:`LineString` coordinates from
+    center coords, surface angles and length of line.


This function is not on the doc, should it?

Since it's a helper function I'll make it private! thx

cedricleroy · 2019-07-31T19:53:17Z

pvfactors/geometry/base.py

+    positive, rotation angles can be negative.
+    In pvfactors, positive rotation angles will indicate pvrows pointing to the
+    left, and negative rotation angles will indicate pvrows pointing to the
+    right (no matter what the the axis azimuth is).


Not in the doc?

cedricleroy · 2019-07-31T19:54:28Z

pvfactors/geometry/base.py


+    Parameters
+    ----------


Parameters order to review.

cedricleroy · 2019-08-01T18:44:19Z

pvfactors/engine.py

-        self.is_fast_mode = isinstance(fast_mode_pvrow_index, int) \
-            and fast_mode_pvrow_index < params['n_pvrows']
+        self.pvarray = pvarray
+        self.is_fast_mode = isinstance(fast_mode_pvrow_index, int)


Would personally do this to avoid the type check:

self.is_fast_mode = False if fast_mode_pvrow_index is None else True

cedricleroy · 2019-08-01T19:05:12Z

pvfactors/geometry/pvarray.py


    @classmethod
-    def from_dict(cls, parameters, surface_params=[]):
-        """Create ordered PV array from dictionary of parameters
+    def init_from_dict(cls, pvarray_params, surface_params=[]):


Default list to be avoided.

cedricleroy · 2019-08-01T19:06:40Z

pvfactors/geometry/pvarray.py

+                   surface_params=surface_params)
+
+    @classmethod
+    def transform_from_dict_of_scalars(cls, pvarray_params, surface_params=[]):


Default list to be avoided.

cedricleroy · 2019-08-01T19:09:57Z

pvfactors/geometry/pvground.py

+    @classmethod
+    def from_ordered_shadow_and_cut_pt_coords(
+        cls, x_min_max=None, y_ground=Y_GROUND, ordered_shadow_coords=[],
+            cut_point_coords=[], surface_params=[]):


Default list to be avoided.

ok got it! thx!

cedricleroy · 2019-08-01T19:11:13Z

pvfactors/geometry/pvground.py

+
+    Parameters
+    ----------
+        list_shaded_surfaces : list


is the indent expected?

cedricleroy · 2019-08-01T19:12:25Z

pvfactors/geometry/pvground.py

+
+    Parameters
+    ----------
+        list_shaded_surfaces : list


* http://effbot.org/zone/default-values.htm

anomam added 6 commits July 25, 2019 14:57

Make calculation of PV row coords vectorized

f1dd2fd

Now can create PV rows and ground based on vectorized calculations

d584454

* next is calculating shadows...!

Make sure not trying to transform with index out of range

6cccc4c

Added test for 2D solar vector calculation

310cac5

Vectorized ground shadow coords calculation

201423b

Updated shadow coords calc, now makes more sense

82a035a

anomam added the WIP Work in progress label Jul 25, 2019

anomam self-assigned this Jul 25, 2019

anomam added 20 commits July 26, 2019 11:30

Order the shadow coords, and calculate when direct shading happens

4464674

Leverage numpy indexing for pv row coordinates

7719186

Finally able to create ground shadows from vectorization

6780246

Can calculate cut points vectorized

b4e7d8c

Recurse to create ground surfaces with cut points and shadows

1900588

Fix calculation of cut point coordinates: needed radians for rotatin

0ae919e

Add missing pieces from other class: interrow shad, neighbrs, views

834a1b2

Started formatting OrderedPVArray to the fit/transform paradigm

303ffbf

Fix class method: all tests in test_pvarray.py pass

474b6f1

Now casting shadows and cutting grnd in transform + private methods

053a1d5

* fixed tests accordingly

Now all pvarray properties will be rebuilt everytime called

5ed70df

* because pvrows and ground can change, so it's hard to know if an update needs to be done or not

Now reflectivities passed to irradiance models at init

6a4557c

New pv array class methods and use private methods as well

8926855

Update engine to run pv arrays with new fit/transform paradigm

0737fc7

fix pvarray vfcalculator and models tests

36cd963

Fix edge point calculation issue and run fn tests

58fec1e

* the edge pts list was not re-initialized at every interation

Quick fix for projection approximations errors in fast class

6a2b6f8

Make direct shading condition consistent with previous class

1bc1a74

Add TODO in test

bffff14

anomam commented Jul 30, 2019

View reviewed changes

anomam added 6 commits July 30, 2019 19:03

Start cleaning up

5ea1bba

Clean up engine.py docstrings and PVEngine class

ee9173f

Clean up geometry/base.py and udpate docstrings

b6964e1

Updated docstring in pvground.py

f1d232a

Update docstrings of OrderedPVArray

252ffaf

Remove old redundant and slower class

43a1fd8

anomam removed the WIP Work in progress label Jul 31, 2019

anomam requested a review from cedricleroy July 31, 2019 09:22

anomam added 4 commits August 1, 2019 15:49

Updated 4/6 tutorials

934deaf

Implement fix in util function to create ground + add test

023e304

Updated tutorials on running timeseries simulations

43e06b1

Update readme and tutorials from sphinx docs section

c8dd679

cedricleroy approved these changes Aug 1, 2019

View reviewed changes

anomam added 2 commits August 2, 2019 11:48

Clean up and add suggestions

6832f12

Do not use mutable objects like [] as fn input default values

9e8fddb

* http://effbot.org/zone/default-values.htm

anomam merged commit c7d0ff1 into master Aug 2, 2019

anomam deleted the vectorize_geometry_calcs branch August 2, 2019 03:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vectorize geometry calculations #60

Vectorize geometry calculations #60

anomam commented Jul 25, 2019 •

edited

Loading

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam Jul 30, 2019

anomam commented Jul 31, 2019

cedricleroy left a comment

cedricleroy Jul 31, 2019

cedricleroy Jul 31, 2019

anomam Aug 2, 2019

cedricleroy Jul 31, 2019

cedricleroy Jul 31, 2019

cedricleroy Aug 1, 2019

cedricleroy Aug 1, 2019

cedricleroy Aug 1, 2019

cedricleroy Aug 1, 2019

anomam Aug 2, 2019

anomam Aug 2, 2019

cedricleroy Aug 1, 2019

cedricleroy Aug 1, 2019

Vectorize geometry calculations #60

Vectorize geometry calculations #60

Conversation

anomam commented Jul 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anomam commented Jul 31, 2019

cedricleroy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anomam commented Jul 25, 2019 •

edited

Loading