xjiang4 / ellipsoids

Automatically exported from code.google.com/p/ellipsoids
Other
0 stars 1 forks source link

Examples of reach set calculation from ET manual fail #94

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There were errors in the examples with nontrivial matrices. In 
s_chapter07_section02_snippet01, s_chapter07_section02_snippet02, 
s_chapter07_section03_snippet01, s_chapter07_section03_snippet02.
Matrices, where all elements are NaN, appeared.

These scripts are located in \doc\mcodesnippets

One needs to 
1) Create the test configurations based on the examples in

\EllTbx\products\+gras\+ellapx\+uncertcalc\+test\+regr\+conf\+sysdef\confrepo\_t
emplates (for system parameters)

and 

\products\+gras\+ellapx\+uncertcalc\+test\+regr\+conf\confrepo\_templates for 
program parameters.

Then these configurations need to be specified in confList in 
elltool.reach.test.run_continuous_reach_tests()

Finally run the tests by gras.ellapx.test.run_tests and 
elltool.reach.test.run_continuous_reach_tests

and make sure that they fail for the newly created configurations.

2) Fix the bugs

Original issue reported on code.google.com by heartofm...@gmail.com on 8 Apr 2013 at 3:13

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 8 Apr 2013 at 10:31

GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 10 Apr 2013 at 8:03

GoogleCodeExporter commented 8 years ago

Original comment by kirill.m...@gmail.com on 10 Apr 2013 at 9:47

GoogleCodeExporter commented 8 years ago
You've committed changes directly into trunk so I had to revert them.

Original comment by heartofm...@gmail.com on 14 Apr 2013 at 8:19

GoogleCodeExporter commented 8 years ago
Good news about the snippets. Once you are done with ReachContinuous please use 
the new properties to fix the example in s_elldemo_reach so that the test in 
elltool.demo.test.mlunit.BasicTestCase('testDemoReach') passes.

Original comment by heartofm...@gmail.com on 21 Apr 2013 at 10:56

GoogleCodeExporter commented 8 years ago
I think you need to add "isRegEnabled" flag to ReachContinuous so that it is 
possible to write 

ReachContinuous(....,'isRegEnabled',true) which will assume that regTol=absTol 
taken elltool.conf.Properties.getAbsTol and isJustCheck=false.

Default value for isRegEnabled should be false.

Original comment by heartofm...@gmail.com on 25 Apr 2013 at 4:41

GoogleCodeExporter commented 8 years ago
One adjustment to the comment #7:

We need to 

1) create a patch for elltool configurations (opened by 
elltool.editconf('default') command). 

2) add elltool.conf.Properties.getRegTol methods that works analogously to 
getAbsTol and others

3) use elltool.conf.Properties.getRegTol as a default value for regTol in 
ReachContinuos

Original comment by heartofm...@gmail.com on 25 Apr 2013 at 5:08

GoogleCodeExporter commented 8 years ago
Comment about 'example.m'.
regTol >= 0.006 - Ok
regTol in [0.001; 0.005] fails with:
Error using gras.ode.ode45reg (line 250)
Oops, we shouldn't be here, regularization haven't worked
regTol <= 0.0009 fails with:
Index exceeds matrix dimensions.
Error in gras.gen.MatVector.rMultiplyByVec (line 76)
                Bpt_data(:,t)=Bt_data(:,:,t)*pt_data(:,t);

Original comment by heartofm...@gmail.com on 26 Apr 2013 at 3:06

GoogleCodeExporter commented 8 years ago
In my opinion now I only need to add try-catch block in ReachContinuous, all 
other tasks are made.

Original comment by kirill.m...@gmail.com on 26 Apr 2013 at 9:43

GoogleCodeExporter commented 8 years ago
Also, I need to use regularization in new refine method. Am I right?

Original comment by kirill.m...@gmail.com on 26 Apr 2013 at 9:55

GoogleCodeExporter commented 8 years ago
It should be done automatically as refine uses makeEllTube which uses 
regularization, right?

Original comment by heartofm...@gmail.com on 26 Apr 2013 at 10:21

GoogleCodeExporter commented 8 years ago
No, now regularization is made in constructor. I will move it to makeEllTube.

Original comment by kirill.m...@gmail.com on 27 Apr 2013 at 12:43

GoogleCodeExporter commented 8 years ago
It seems like I need to add try-catch block in self.makeEllTubeRel because 
refine and evolve methods also can provide regularization errors. Do you agree?

Original comment by kirill.m...@gmail.com on 27 Apr 2013 at 2:23

GoogleCodeExporter commented 8 years ago
1) In catch block please 

a) rename exception into meObj
b) Avoid the copy-pasting of error messages by creating constants for each
typical message, right now you have very similar messages like

'There is problem with regularization ',...
                        'tolerance, try to increase it.'

'Probably you need to use regularization.'
'Try to use regularization.'

This does make much sense to me, the messages should be clear and distinctive...
c) Same with errorTags - you have

wrongInput:badRegTolODE45Failed
wrongInput:badRegTol

what is the difference if the advice is similar - use regularization. 
If you need to distinguish these tags - at least rename

wrongInput:badRegTolODE45Failed into wrongInput:badRegTol:ode45Failed

to stress that wrongInput:badRegTol:ode45Failed is a specific case of 

wrongInput:badRegTol

Same problem with wrongInput:onlyCheckEnabled - it is also regularization 
problem
but nothing in the identifier indicates that.

I suggest you use something like this:

wrongInput:regProblem:regTolIsTooLow
wrongInput:regProblem:onlyCheckIsEnabled

etc. i.e. you need a clear error tag hierarchy and please create the constants
for error tags(indetifiers) to avoid the copy-pasting

d) Use 

if ...

elseif ..
...

elseif ..
...
else
...
end

instead of 

if 
end
if 
end

e) In the error messages you should be more descriptive and explain what a user 
needs to do. For instance, if you say that a user needs to 
increase regTol - please explain what property needs to be used to set up 
regTol. If you say about initial ellipsoid - do the same and explain
which parameter needs to be changed etc. Think of it as a tool for dummies. 

2) Are you use that we need that many projections and initial directions
for snippet xml configurations and other configurations you've added?

After all we want to replicate the same calculation that we do in the snippets 
so we need to make
sure that we build no unnecessary projections/tubes. Calcualtion of all the 
tubes and projections
might not be that slow but plotting of all the graphs is quite slow.

Btw - snippet722 test has passed

Original comment by heartofm...@gmail.com on 8 May 2013 at 11:54

GoogleCodeExporter commented 8 years ago

Original comment by kirill.m...@gmail.com on 8 May 2013 at 4:39

GoogleCodeExporter commented 8 years ago
See comments to your last commit.

Original comment by heartofm...@gmail.com on 9 May 2013 at 12:01

GoogleCodeExporter commented 8 years ago
Also, have you removed the snippet configuraitons. Also, do all the Irina's 
snippet tests pass?

Original comment by heartofm...@gmail.com on 9 May 2013 at 12:02

GoogleCodeExporter commented 8 years ago
1) Please make sure that we have the tests for plot_ia/ea for 2/3 dimensional 
projections and for un-projected set.
2) Why in ETManualTC.m you disable 2 snippets, not just one?
3) Please merge with trunk and make sure that your tests pass.

Original comment by heartofm...@gmail.com on 19 May 2013 at 12:50

GoogleCodeExporter commented 8 years ago
2) I did that because in the first snippet we have cut() for projection and in 
the second - regTol error.

Original comment by kirill.m...@gmail.com on 23 May 2013 at 2:46

GoogleCodeExporter commented 8 years ago
Please specify the names of these snippets in tickets 115 and 120.

Original comment by heartofm...@gmail.com on 23 May 2013 at 3:33

GoogleCodeExporter commented 8 years ago

Original comment by kirill.m...@gmail.com on 23 May 2013 at 7:15

GoogleCodeExporter commented 8 years ago
Provided answer via email

Original comment by heartofm...@gmail.com on 24 May 2013 at 6:32

GoogleCodeExporter commented 8 years ago

Original comment by kirill.m...@gmail.com on 26 May 2013 at 3:40

GoogleCodeExporter commented 8 years ago
Provided answer via email

Original comment by heartofm...@gmail.com on 27 May 2013 at 6:01

GoogleCodeExporter commented 8 years ago

Original comment by kirill.m...@gmail.com on 29 May 2013 at 2:30

GoogleCodeExporter commented 8 years ago

Original comment by heartofm...@gmail.com on 30 May 2013 at 1:13