SystemAnalysisDpt-CMC-MSU
Cover GenEllipsoid, ellipsoid and hyperplane with parameterized test cases and make them use common code where possible
Deadline: end of the semester
The goal of this task is to finish the re-factoring of different classes representing ellipsoids, hyperplanes and make the tests for them parametrized by ellipsoid object factories. Such parametrization allows for running the same number of tests against different kinds of ellipsoids (both generalized and plain) and hyperplanes. Of course not all tests can be so universal but many can. For instance if a test checks a behaviour of "rho" method responsible for support function calculation this test doesn't need to know which object it tests. All this test really needs is a factory that can create objects. This factory can then be passed into the test cases using the approach described here: https://github.com/SystemAnalysisDpt-CMC-MSU/ellipsoids/wiki/MLUNITEXT-unit-testing-framework-description#writing-parameterized-tests.
Branch issue_8_vert_svart implements the described ideas but only partially. The main problem with implementing a test parameterization was not having many methods in GenEllipsoid class that are present in ellipsoid class. This didn't allow for the tests intended for testing ellipsoid methods to run for generalized ellipsoid objects (GenEllipsoid class). The solution was to implement all missing methods but this task was not finished. Now there is an understanding that we need to approach this problem in two steps.
- The very first step should be a) creating a copy of issue_8_vert_svart for your task (i.e. you should branch from issue_8_vert_svart, not from master). b) rebasing your branch to master and making sure that tests are successfully run on the server. Most of the tests won't pass but the tests runs should not crash so that we can see the test results.
Only when this is done we should move to the next step. Please note - it will be much more easier to rebase if after creating your branch as a copy of issue_8_vert_svart you combine all commits starting with "68fe72c9f1c98d81d91451fd0dba7b95e012cbb9" made on 08-Mar-2015 into a single commit (open log, select all commits you want to combine, then right click - > combine to one commit). If you start rebasing without combining all commits into one you will have to resolve much more conflicts. Yes, there will be a lot of conflicts but you need to go through the changes very carefully and resolve each conflict manually. In 99% of cases it will be clear how to resolve them because the changes in issue_8_vert_svart branch are very typical and similar to each other.
- The next step should be providing a simplified implementation of all missing methods from ellipsoid in GenEllipsoid class.
A generalized ellipsoid is a set that can be represented as sum E+L, where L - a subspace of R^n and E is some (may be degenerate) ellipsoid in an orthogonal complement to L. A generalized ellipsoid doesn't have a configuration matrix in a common sense (by construction) but it does have a) a basis in L and its complement. b) a configuration matrix of E (ellipsoid in the orthogonal complement to L)
Since all tests for "ellipsoid" class (plain ellipsoid) only deal with the case of empty L it will be sufficient to implement the missing methods of GenEllipsoid via "ellipsoid" class. Here an an example of how some hypothetical method "methodX" that is implemented in "ellipsoid" class can be implemented GenEllipsoid:
classdef GenEllipsid
methods (Access=private) function shMat=getShapeMatIfPlan(self) %this method returns a shape matrix of generalized ellipsoid self in case L is empty i.e. when self is "plain" ellipsoid. %in case L is not empty getShapeMatIfPlan throws an exception. end end
methods
function varargout=methodX(self,varargin)
%this is a stub method that works only in a very specific case - when self is plain ellipsoid.
shMat=self.getShapeMatIfPlan();
cVec=self.getCenterVec();
tmpEllObj=ellipsoid(shMat,cVec);
if nargout==0
tmpEllObj.methodX(varargin{:});
else
varargout=cell(1,nargout);
[varargout{:}]=tmpEllObj.methodX(varargin{:});
end
end
end end
Method getShapeMatIfPlan is not implemented just yet but it is very easy to implement.
As you can see such implementation of methodX is just a stub which works only when generalized is not actually a generalized ellipsoid but a plain ellipsoid. However this is more than sufficient for making all the existing test pass for both plan and generalized ellipsoids.
Please note that the following methods are common for both GenEllipsoid and ellipsoid so they need to be moved to AEllipsoid class:
move2origin getAbsTol getRelTol gt,lt,ge,le getShape getMove2Origin getNPlot2dPoints getNPlot3dPoints getInv
- The final step step is finishing test parametrization by factories. Once you are done with step 1) you may discover that everything is already parametrized but there is no guarantee. The goal is to have the same tests for both "ellipsoid" class, "GenEllipsid" class and sometimes - "hyperplane". Tests need to be partitioned into the following groups
a) tests that are specific to GenEllipsid - these tests cover generalized ellipsoid functionality and not applicable for "plan" ellipsoids b) tests that are specific to hyperplanes ("hyperplane" class) c) tests that are common for both GenEllipsid and ellipsoid classes. d) tests that are common for GenEllipsid, ellipsoid and hyperplanes. Yes, there are tests that are suitable for both hyperplanes and ellipsoids.
Each of these groups should consist of one or more tests cases parametrized by ellipsoid/hyperplane factories. Please note that this is almost done in the issue_8_vert_svart, you just need to finish what was started.
One more thing to keep in mind - the changes introduced in issue_8_vert_svart are very conflict-provoking which means that even after resolving all the conflicts and fixing all the problems a single change in "master" branch can cause new conflicts. That is because people working with master do not know about test parametrization introduced in issue_8_vert_svart so thay continue writing new non-parametrized tests. So the sooner you finish the task - the fewer conflicts you will have to resolve.
Finally, please note that issue_8_vert_svart slightly changes a hierarchy of ellipsoid classes by introducing AEllipsoid and ABasicEllipsoid classes. elltool.core.ABasicEllipsoid is a base class for all kinds of ellipsoids and hyperplane.
Please rename ABasicEllipsoid into AGeomBody.
AEllipsoid is inherited from ABasicEllipsoid and is a base class for both ellipsoid and GenEllipsid, while hyperplane is inherited directly from ABasicEllipsoid.
Am I right that first of all I should resolve all of conflicts in process of rebase, push the branch after that and wait for results of tests on server?
I rebase now and I usually meet function "ell_inv". But I can't find description of this function in the branch "issue_8_vert_svart". Is it correct function or I should avoid using of it, using gras.geom.ell.invmat istead?
Nikita Zuyanov replaced ell_inv with gras.mat.invmat function. You should use the new function instead of ell_inv
OK, thank you! About the non-obvious moments in the time of rebase. I have resolved majority of conflicts but I don't know what to do with elltool.core.GenEllipsoid.GenEllipsoid - there are too many different changes and conflicts. It looks like functions have been totally rewrited. Which version is more actual? Or should I try to make some ssort of manual cherry-pick?
There is no a general recipe, you need to understand each change and resolve conflicts manually.
Please note - you won't be able to resolve conficts if you do not try to understand the changes.
It is not "too many", it is just "many" and the task is perfectly doable. "Many" is because this task is not warm-up task but a task for the whole semester.
Answering your question about cherry-picking. All I need is the final result which is a branch that contains both changes from issue_8_vert_svart and master. I suggested you the shortest path as I see it - to resolve all conflicts, one by one.
However, you have a permission to use an alternative approach - branch from current master and apply all changes from issue_8_vert_svart by hand or via cherry picking. I just need the result. Choice is yours, whatever works best for you. What I do not want is having certain features, implemented in issue_8_vert_svart, not finding their way into the resulting branch. I.e. if you do everything by hand or via cherry-picking - make sure that you do not miss any changes from issue_8_vert_svart.
Just let me know which way you want to proceed.