SystemAnalysisDpt-CMC-MSU / ellipsoids

Ellipsoidal Toolbox for MATLAB is a standalone set of easy-to-use configurable MATLAB routines and classes to perform operations with ellipsoids and hyperplanes of arbitrary dimensions
http://systemanalysisdpt-cmc-msu.github.io/ellipsoids
Other
20 stars 7 forks source link

Refactor comparison methods of GenEllipsid, ellipsoid and hyperplane via inheriting these classes from modgen.common.obj.HandleObjectCloner #44

Open pgagarinov opened 8 years ago

pgagarinov commented 8 years ago

Deadline: end of the semester

GenEllipsoid, ellipsoid and hyperplane classes are all inherited from elltool.core.AGenEllipsoid class that implements isEqualInternal method that is designed for an element-wise comparison of two object arrays. HandleObjectCloner in its turn takes this even futher by introducing separate methods for element-wise comparison, pair-wise comparison of multiple arrays and even supporting gt,le,lt,ge and sort operators based on lexicographical comparison of BLOB sequences. The goal of this task is to get rid of isEqualInternal method by replacing it with the methods inherited from HandleObjectCloner class. Doing so will cause the following changes in the way some of the comparison methods work:

I) isEqual method implemented in AGenEllipsoid now returns a logical array isEqualArr that contains a result of element-wise comparison of corresponding object elements while isEqual implemented in HandleObjectCloner returns a scalar logical output that contains a result of comparison of multiple inputs (type edit modgen.common.obj.HandleObjectCloner.isEqual to see for yourself). However HandleObjectCloner.isEqualElem does the same as AGenEllipsoid.isEqual. This means that after inheriting AGenEllipsoid from HandleObjectCloner all tests (and other code) that used AGenEllipsoid.isEqual to do element-wise comparison will have to be changed to use isEqualElem. The good thing however is that most of the code while calling isEqual did it solely for scalar objects which means this code expected an output of isEqual to be scalar. Thus most of the code won't have to be changed.

II) AGenEllipsoid uses Matlab built-in implementation of isequal, isequaln and isequalwithequalnans which is based on comparing objects as binary sequences. HandleObjectCloner however changes that by redirecting calls to these 3 methods to either isEqualScalarInternal or isEqualScalarAsBLOBInternal depending on what HandleObjectCloner.getComparisonMode method returns (if asBLOB property is not specified). There is asHandle flag that can also be specified, if true then calls to isequal, isequaln and isequalwithequalnans are redirected to eq with asHandle=true flag and objects are compared as handles.

III) HandleObjectCloner also re-defines >=, <=,>,< == and ~= operators (same as ge, le,gt, lt,eq,ne). Since ellipsoid overrides <=,>=,<,> based on its own ellipsoid-based logic an implementation of this operators in ellipsoid class should not be touched. However, ne operator in ellipsoid class needs to be removed since HandleObjectCloner's implementation of the same operator is more correct.

IV) Some tests for ellipsoid, hyperplane and GenEllipsid classes use eq,==, isequal, isequaln and isequalwithequalnans methods to compare ellipsoids. There can be two reasons for that a) a need to compare objects as binary sequences (this is the way the built-in implementation of isequal* methods work) i.e. with absolute precision using all content of the object including the fields like "absTol" that ordinary "isEqual" method does not compare. b) Just a human error - a developer could have used isEqual but used isequal instead just because it looked more familiar. These two case need to be treated separately - in case a) you need to replace calls with isEqual with "asBlob" flag while in b) you need to use just "isEqual" without asBlob.

The complete functionality of HandleObjectCloner can be easily studied by looking at the tests of this class in +modgen+common+obj+test+mlunit\HandleObjectClonerTC.m test case.

Here is what you need to do step by step

1) Remove implementation of isEqualInternal method in AGenEllipsoid and add inheritance from modgen.common.obj.HandleObjectCloner. 2) Go through all tests for ellipsoid, GenEllipsid and hyperplane classes in elltool.core.test. package and make a method replacement based on items I)-IV) and Make sure all tests from elltool.core.test package pass. 3) Make sure that all ET tests pass. 4) Implement additional tests in a separate test case class in elltool.core.test.mlunit package for isEqualElem, isequal, isequaln,isequalwithequalnans, ne and eq methods. These tests should be made common for ellipsoid, hyperplane and GenEllipsid i.e. parametrized by these objects factory (i.e. one test case class should test all 3 classes). When doing so please refer to elltool+core+test+mlunit\HyperplaneDispStructTC and elltool+core+test+mlunit\EllipsoidDispStructTC test cases as example (both these test cases are inherited from elltool.core.test.mlunit.ADispStructTC class that contain the tests while sub-classes define certain methods that contain all the differences between of hyperplanes and ellipsoids).

pgagarinov commented 8 years ago

You probably will find it useful to talk to Kirill about HandleObjectCloner - he also has an experience of using this class.