Make ReachContinous return an original system dynamics even for a system in a backward time (for a attainability domains)

[pgagarinov]

Deadline - 1st of November, by that date the code with all the tests should be in "master" branch.

Currently evaluating an solvability tube as per the following example

aMat = {'2_sin(t)' '0'; '0' '2_sin(t)'}; bMat = eye(2);
SUBounds = struct();
SUBounds.center = {'0'; 't/10'};
SUBounds.shape = [.1 0; 0 .1];
sys = elltool.linsys.LinSysContinuous(aMat, bMat, SUBounds);
x0EllObj = ell_unitball(2);
timeVec = [2 0];
dirsMat = [1 0; 0 1]';
rsObj = elltool.reach.ReachContinuous(sys, x0EllObj, dirsMat, timeVec);
rsObj.evolve(-1,sys);

would result into modifying the system parameters so that A(0) is no longer [2 0;0 2]:

pdl = rsObj.getIntProbDynamicsList;
pdl1 = pdl{1}{1};
at = pdl1.getAtDynamics();
at.evaluate(0)

ans =
-1.8186 0
0 -1.8186

This is because "prepareSysParam" method in ReachContinous transforms the system parameters in a such way that a reachability tube for the modified system equals a solvability domain for the original system in an opposite time.

One needs to make sure that modifying the system parameters becomes an internal business of ReachContinous while the system dynamics returned by getIntProbDynamicsList method (and others) corresponds to the original (non-modified) system.

The way to implement this is to create a class that would be a "wrapper" system for any system in forward time. This way instead of manually modifying each system matrix (A(t), C(t), B(t) etc) in prepareSysParam we would just create an instance of the wrapper system like this

backTimeWrapperSysObj=BackwardTimeWrapSystem(sysDynObj);

and then would use this backTimeWrapperSysObj internally while externally getIntProbDynamicsList method and others would return the original dynamics no matter what.

The new wrapper class needs to be placed into products+elltool+linsys package next to other LinSys classes and separately covered with tests in a new test case placed into products+elltool+linsys+test+mlunit package.

Obviously we will have to have 2 different wrapper classes for both discrete and continuous systems.

Also the correct behavior of ReachContinuous and ReachDiscrete classes needs to be covered with
a few tests
\products+elltool+reach+test+mlunit

Finally all the existing tests need to be fixed because in some places it is currently assumed that the getIntProbDynamicsList works the way it works now so those places will have to be fixed so that all the tests pass.

[shm512]

After several days of looking at the code of ReachContinious and AReach classes, I still can't understand several things:

1. What is the meaning of "gtStrCMat" and "ctStrCMat"? The first one occurs only in prepareSysParam, while the second one -- only in getProbDynamics... Is it the same thing (constructor of AReach treat them like this, so in my current wrapper I did the same, and it seems to work)?
2. Can I modify only ReachContinious, or AReach too ("modifying the system parameters becomes an internal business of ReachContinous")? Then one of possible solutions is to make getProbDynamics return two gras.ellapx.lreachplain.probdyn.LReachProblemDynamicsInterp objects instead of one (first for makeEllTubeRel -- it should be wrapper object -- and second for intProbDynList and extProbDynList)... or the same for prepareSysParam.

[pgagarinov]

1. gt is G(t) in \dot(x)=A(t)x(t)+B(t)P(t)+G(t)Q(t), i.e. a transformation matrix for a disturbance constraint.
2. Well, AReach provides generic functionality for both ReachContinuous and ReachDiscrete. The wrapper objects should be different for discrete and continuous systems so some of your changes should be made in ReachContinuous, some - in AReach and some - in ReachDiscrete.

I think that adding a second output for an existing method is a good idea. Let make this method return original dynamics (not modified) as the first output and modified - as a second. Some existing tests expect a modified dynamics as the first output so those tests need to be slightly modified so that they use a second output. Some of the tests perform a transformation upon transformation to restore the original dynamics. Such tests needs to be slightly modified so that they just use the first output wihout any additional transformations.

Now about ReachDiscrete. The reason ReachDiscrete wasn't mentioned in the task description in the first place is because for a discrete systems there are already two implementations of ProblemDynamics objects (see implementation of "getSysWithDisturb" in ReachDiscrete) one for the forward time and one - for the opposite time. Thus for ReachDiscrete the solution is a bit simpler - you just construct both dynamics instead of constructing either forward or backward dynamics and return them as different outputs from getIntProbDynamicsList and use only the second output while ReachDiscrete and ReachContinuous should provide their specific implementation of this method.

Please let me know if this doesn't answer your question or if you have any other questions.
Thanks.

Thus you need to modify ReachDiscrete, AReach and ReachContinuous. AReach should now expect two outputs from

[shm512]

OK, code has been implemented and tested on sample from issue description (only had to change first line, because in description it's incorrect). Probably this test should be added to test coverage? But I don't quite understand hierarchy of tests in elltool/+reach/+test/+mlunit. Should my test extend some of the test classes there or just mlunitext.test_case (with a single assert on the end in that case)?

[pgagarinov]

1. Please study the way elltool.reach.test.run_cont_tests and
   elltool.reach.test.run_discr_tests function work. You will need this
   for the next final task anyways so this won't be a waste of time. The idea is that the most of the
   test cases are parameterized and called for different systems
   in both forward and backward time. Just go through all the test cases
   in run_cont_tests one by one and see how it works.

Please note that you when ALL the tests are put into a single test suite
line 138: suite = mlunitext.test_suite(testList);
they are not run just yet. At that time we can filter them by keeping
only those that we really want to run (getCopyFiltered call).

See a help header for run_cont_tests for examples that show how to run a particular test
from one or all test cases. Since the same test from the same test case can be run for different
parameters some tests (that are duplicated) have a market that is indicative of the parameter
combination that these tests are run for. Market can be passed as the first parameter
to both run_cont_tests and run_discr_tests.

Finally some test cases are run for both discrete and continuous systems, for instance the following
two test cases have the same parent that contains tests that run for both continuous and discrete systems,
in other works - tests from elltool.reach.test.mlunit.ATestDynGettersBaseTestCase are system type-independent.

DiscreteSecReachTestCase < elltool.reach.test.mlunit.ATestDynGettersBaseTestCase
ContinuousSecReachTestCase < elltool.reach.test.mlunit.ATestDynGettersBaseTestCase

I suggest you put your tests into elltool.reach.test.mlunit.ATestDynGettersBaseTestCase
and then run your test like this

elltool.reach.run_cont_tests('.','.','myTestName');
elltool.reach.run_discr_tests('.','.','myTestName');

These commands will run your tests for all kinds of systems.

Please note that there is no need to create a reach object in your test - will automatically be available
in self.reachObj field where it is put by "set_up_param" method.

I think your test should check a) the first output is the dynamics in forward time
b) the second output depends on wheather the system is forward or backward time.

2. Probably this test should be added to test coverage?
   Rather yes but please describe in more details what this test will check?

[pgagarinov]

Please rename your branch using your real name instead of account name

[pgagarinov]

Please delete branch issue_17_shm512

[shm512]

Can you please explain what function isEqualBPBandpDynBPBatCurTime(pDynBPBMat,curTime,iLinSys) in elltool.reach.test.mlunit.ATestDynGettersBaseTestCase does in test case elltool.reach.test.mlunit.ContinuousSecReachTestCase/testBaseTestDynGetters[demo3fourthTest_IsBackTrueIsEvolveTrue]?
This test is the only one that fails with my changes, but I can't understand how could it be: the first return value (probDynActionObj) of method getProbDynamics (of class elltool.reach.ReachContinious) hasn't changed, while the added second one (probDynOriginObj) is only used in setting intProbDynList and extProbDynList in ReachContinious. I'd tried to set them also by propDynActionObj (for testing only), but this test still failed.
Therefore the only possible reason for this test's failure can be incorrect creation of probDynActionObj in getProbDynamics. But I need to understand what this test exactly does to figure out what's wrong with it. I've traced the failure of the test to the mentioned function, but I've failed to understand what this function does.

Stack trace:
+elltool+reach+test+mlunit\ATestDynGettersBaseTestCase.m at line 168 (isEqualBPBandpDynBPBTimeInterval)
+elltool+reach+test+mlunit\ATestDynGettersBaseTestCase.m at line 138 (isEqualBPBandpDynBPB)
+elltool+reach+test+mlunit\ATestDynGettersBaseTestCase.m at line 79 (auxtestProbDynGetters)
+elltool+reach+test+mlunit\ATestDynGettersBaseTestCase.m at line 54 (testBaseTestDynGetters)

[pgagarinov]

To keep it brief - this function compares B_P_B.' extracted from probDynObj returned by getIntProbDynamicsList and getExtProbDynamicsList with the same matrix extracted from a list of linear system definition objects called linSysList (the same list passed as input to both ReachContinuous and ReachDiscrete). This list is returned by getSystemList method of ReachContinous/Dynamics. The difference between a system definition object and a problem dynamics object is that the latter contains methods for calculating expressions like B(t)_P(t)_B(t).' that are used directly in ellipsoidal approximation formulas and control synthesis. The best way to see a difference is to compare a linear system definition interface
+gras+ellapx+lreachplain+probdef\IReachContProblemDef.m
and system dynamics interface +gras+ellapx+lreachplain+probdyn\IReachProblemDynamics.m. Please note that these interfaces are for systems without uncertainty, the extended interfaces for systems with uncertainty are located in gras.ellapx.lreachuncert package.

[shm512]

I can't understand why linSysCVec (which is returned by getSystemList method) is different from probDynOriginObj (which is returned by getProbDynamics), except for their different class. Both of them are set by linSys parameter of constructor of class AReach. I'm still trying to change it from different places in code - probably that's the only thing I can do, because I still can't understand how this test works.

[pgagarinov]

The best way is to use a debugger and conditional breakpoints. Please keep trying - I can answer questions but you need to do it by yourself. You can talk to Anastasia Semenova (if she is still around) who is an author of these tests.

[shm512]

What's the meaning of "BPBTransDynamics" property of class gras.ellapx.lreachplain.probdyn.AReachProblemDynamics (specifically its descendant gras.ellapx.lreachplain.probdyn.LReachProblemLTIDynamics)? Should it be based on system used in calculations or original system?
And, finally, was I right to think that "intProbDynList" and "extProbDynList" properties aren't used in calculations, so they should be filled with original system? It seem now to me that l wasn't...

[pgagarinov]

What's the meaning of "BPBTransDynamics" property of class

BPBTransDynamics object represents B(t)_P(t)_B^{'}(t) matrix function where B(t) is from
\dot(x)=A(t)x+B(t)u+....
and P(t) is a shape matrix for control bounding ellipsoid. In All the formulas we never need neither B nor P separately, only B(t)_P(t)_B^{'}(t), that is why we have an object that represents this matrix.

Should it be based on system used in calculations or original system?

Encapsulation OOP concept assumes that classes do not know anything about the way they are used.
This means that LReachProblemLTIDynamics class represents just "a system", you can use it to store any system including the original one and the transformed one. All depends on what data you pass into the constructor. Just keep in mind that LReachProblemLTIDynamics repesents a system with a constant matrix only, there are other derivatives of AReachProblemDynamics for a generic A(t).

was I right to think that "intProbDynList" and "extProbDynList" properties aren't used in calculations, so they should be filled with original system?

Both intProbDynList and extProbDynList are not used in any calculations inside of AReach, at least that was an idea. These properties are there for a record so that class users can get them to see which systems an ellipsoidal approximation correspond to. And I think your idea to make the corresponding getters return two outputs instead of one was a good one.

[shm512]

In spite of all my attempts (I've completely rewritten my edits several times), this test (elltool.reach.test.mlunit.ContinuousSecReachTestCase/testBaseTestDynGetters[demo3fourthTest_IsBackTrueIsEvolveTrue]) still fails. The only difference causing test failure is between
bpbMat =
0.000010000000000 0 0 0
0 0.000010000000000 0 0
0 0 11.111111111111109 0
0 0 0 6.250000000000000
and pDynBPBMat =
0 0 0 0
0 0 0 0
0 0 11.111111111111109 0
0 0 0 6.250000000000000
in isEqualBPBandpDynBPBatCurTime function of elltool.reach.test.mlunit.ATestDynGettersBaseTestCase.m

I have only two idea about possible reasons:

1. That test is somehow wrong.
2. intProbDynList and extProbDynList are modified in evolve (AReach.m: lines 1817-1818) by probDynObjCell (there it is named as intProbDynCell and extProbDynCell) output of evolveApprox, which is set (AReach.m: line 549) by probDynObj output of makeEllTubeRel. So I made makeEllTubeRel return also probDynOriginObj just for this case. In this case that test succeed, but two other test fails:
   elltool.reach.test.mlunit.ContinuousReachRegTestCase/testRegularization
   elltool.reach.test.mlunit.ContinuousReachProjTestCase[demo3fourthTest]/testSystem
   And one more test got error:
   elltool.reach.test.mlunit.ContinuousReachTestCase[demo3fourthTest_IsBackTrueIsEvolveTrue]/testGetCopyAdvanced
   So either reason 2) is wrong (and, therefore, right reason is 1)) or I modified makeEllTubeRel wrongly (but how could that be? edits are minimal!)

Consequently I still have no ideas what to do with it. The only thing I can do - move to issue 38.

[pgagarinov]

3. Making getIntDynamicsList and getExtDynamicsList different dynamics objects was a more significant change that you though because obviously an external code (tests is an external code) made certain assumptions about content of these objects. I suggest you move gradually step by step like this:

• Start from no changes state and start applying your changes in the following order
• Add one more output to the getters and make them return BOTH active and original dynamics (as the second output)
• Make sure that all tests pass. Some tests may fail because you change the code and some exceptions produced inside AReach and ReachContinous will have different identifiers. Certain tests like testRegularization check identifiers of error messages so you need to change those tests of fix problems in your code before moving to the next step.
• Once all the tests pass you swap the output arguments (so that actual dynamics comes first)
  and apply the fixes you made to testDyn...Getters test.
  If there are some failures in other tests - fix the tests.
• Remove the second output in the getters.

4. Instead of "if ~isOk error('ext(2)")
   please just add the third output to asset and assert_equals method:

mlunitext.assert_equals(true, isOk,"Something is wrong, bla bla bla"0)

5. Please never commit garbage code and commented code, if you delete something - delete it completely (for instance prepareSysParams overloading should now be removed altogether).

6. If you need to output something to console for debugging - please use log4j (https://github.com/SystemAnalysisDpt-CMC-MSU/ellipsoids/wiki/Logging-with-log4j with debug method.

P.S. You can work on your next task in parallel but this one needs to be finished anyway.

[shm512]

Making getIntDynamicsList and getExtDynamicsList different dynamics objects was a more significant change

That's not what I'd done. However, now it doesn't matter, as I rewrited all code once more, and finally made it pass. That's because now I used different semantic: getIntProbDynamicsList and getExtProbDynamicsList return two gras.ellapx.lreachplain.probdyn.LReachProblemDynamicsInterp objects: again, for calculations and for gaining original system elements.
My numerous fails were caused by wrong assumption that getIntProbDynamicsList and getExtProbDynamicsList are used only for second purpose. Renunciation of this assumption lead to success right away.

[pgagarinov]

Right, not now the question is - can we
a) make the "action" output the second one instead of the first one?
b) make all tests use the first output i.e. the original object. Then finally we can just remove the second output.

Please note that the goal of this task is to make the "action" object the internal business of Reach classes. The way it is right now all the tests still use action object which should not be a part of the tested interface. This is what we need. We need to make only 'original' object a part of the public interface and keep 'action' object internal.