StochasticAnalytics / emClarity

GNU Lesser General Public License v3.0
41 stars 6 forks source link

gridCoordinates - single responsibility principle #110

Closed thomasfrosio closed 4 years ago

thomasfrosio commented 4 years ago

BH_multi_gridCoordinates.m - single responsibility principle

At the moment, BH_multi_gridCoordinates.m has to much responsibility, slowing things down and reducing clarity. Below is an example showing how we could refactor the code. This is not backward compatible.

EMC_multi_gridCoordinates: This function should only be used for creating grids later used for interpolation. The interpn function can accept the vectorCoordinates, which allows to reduce the computation on the grids.

As a result, the transformation consists only into a matrix multiplication between the rotm and the coordinates.

EMC_multi_gridMasks: This function take over the coordinates systems part of BH_multi_gridCoordinates and should therefore be used to create masks/filters.

EMC_multi_vectorCoordinates: Compute the vectorCoordinates. Both EMC_multi_gridMasks and EMC_multi_gridCoordinates use this function.

This pull request is just to illustrate the improvements that could be done on BH_multi_gridCoordinates. Using these new functions require some important changes to the rest code, as the inputs are different and some conventions were changed (flgShiftOrigin for example). If you agree with some ideas, I can modify BH_multi_gridCoordinates while keeping backward compatibility, but I think it would be beneficial to split it into multiple functions.