remove function calls to SVD #614

PeterPablo · 2015-04-10T14:57:04Z

introduce sub function that returns a small data table
all svd calls are replaced
this should hopefully partially resolve issues, observed e.g. in introduce font encoding #590 and change float format #604

Should partially fix #414.

* introduce sub function that returns a small data table * all svd calls are replaced * this should hopefully partially resolve issues, observed e.g. in matlab2tikz#590 and matlab2tikz#604

PeterPablo · 2015-04-10T15:21:01Z

Failing tests:

--- mine/test25-converted.tex
+++ (clipboard)
@@ -12,7 +12,7 @@
 \begin{tikzpicture}

 \begin{axis}[%
-width=0.95092\figurewidth,
+width=0.75\figurewidth,
 height=\figureheight,
 at={(0\figurewidth,0\figureheight)},
 scale only axis,

Unfortunately width is determined differently. I raised this issue in #552 (comment).
This test is marked as unreliable in develop.

--- mine/test63-converted.tex
+++ (clipboard)
@@ -39,46 +39,46 @@
 }]
 table[row sep=crcr] {%
 x  y\\
-       7          0\\
-       7         37\\
-      32         37\\
-      32          0\\
-      32          0\\
-      32         15\\
-      57         15\\
-      57          0\\
-      57          0\\
-      57          8\\
-      82          8\\
-      82          0\\
-      82          0\\
-      82          3\\
-     107          3\\
-     107          0\\
-     107          0\\
-     107          3\\
-     132          3\\
-     132          0\\
-     132          0\\
-     132          3\\
-     157          3\\
-     157          0\\
-     157          0\\
-     157          1\\
-     182          1\\
-     182          0\\
-     182          0\\
-     182          1\\
-     207          1\\
-     207          0\\
-     207          0\\
-     207          0\\
-     232          0\\
-     232          0\\
-     232          0\\
-     232          1\\
-     257          1\\
-     257          0\\
+     9.5          0\\
+     9.5         37\\
+    29.5         37\\
+    29.5          0\\
+    34.5          0\\
+    34.5         15\\
+    54.5         15\\
+    54.5          0\\
+    59.5          0\\
+    59.5          8\\
+    79.5          8\\
+    79.5          0\\
+    84.5          0\\
+    84.5          3\\
+   104.5          3\\
+   104.5          0\\
+   109.5          0\\
+   109.5          3\\
+   129.5          3\\
+   129.5          0\\
+   134.5          0\\
+   134.5          3\\
+   154.5          3\\
+   154.5          0\\
+   159.5          0\\
+   159.5          1\\
+   179.5          1\\
+   179.5          0\\
+   184.5          0\\
+   184.5          1\\
+   204.5          1\\
+   204.5          0\\
+   209.5          0\\
+   209.5          0\\
+   229.5          0\\
+   229.5          0\\
+   234.5          0\\
+   234.5          1\\
+   254.5          1\\
+   254.5          0\\
 };
 \addplot [color=black,solid,forget plot]
   table[row sep=crcr]{%

Different coordinate locations. Any ideas? This test is marked as unreliable in develop.

PeterPablo · 2015-04-10T15:27:24Z

How should we proceed?

okomarov · 2015-04-11T13:31:36Z

I reckon there are two things to do first:

Define width and height explicitly as in (but without the float format change) PeterPablo@9e31d1b
Remove instances of hist(). TMW is deprecating it and one reason is inconsistent behaviour (wathever that means)

Basically, this PR will then address partially #552 and #414.

PeterPablo · 2015-04-11T13:39:45Z

Sounds reasonable. Good point especially about removing hist().
In order to introduce explicit width and height we have to adapt PeterPablo@9e31d1b to not pass latex variables, but rather directly supply dimensions -- otherwise the width still contains a calculated value.
So, should I first focus on this PR?

okomarov · 2015-04-11T14:23:32Z

I think so, but I am afraid is gonna be hard 😅 now that I realize what it entails.

PeterPablo · 2015-04-11T14:30:11Z

:-/ A lot more hash calculation fun... I really hope it is worth the effort and we are rewarded by testsuite stability. Actually I would not remove calls to hist() here, but rather do this as a second PR, to not introduce too many changes in one go. I will though (re)label them as unreliable and link to your comment.
Furthermore I will adapt the above linked commit of mine to incorporate fixed dimensions. * crossing fingers this does not introduce new issues *

PeterPablo · 2015-04-11T14:31:11Z

Oh, I'll wait for a short comment of @egeerardyn, so we agree on course of action.

egeerardyn · 2015-04-13T08:13:13Z

With regards to hist, I'm on the one hand very happy that TMW is deprecating it (it has caused a lot of indeed inconsistent behavior, also in our codebase: the internals of hist are a terrible mess).

Anyhow, I think we should keep our limited hist support at least for one more year. Most people I know have not migrated to R2014b, let alone R2015a (where hist has been deprecated), so we still need to keep their condition in mind. E.g. here in my lab it is unfeasible to migrate completely to R2014b since some of our own toolboxes are still not compatible and I can imagine that other labs have similar issues.

When (not if) we remove hist, it should happen in a different PR, regardless of the actual time when we do this.

Regarding the axes dimensions: I think that we always calculate their size, as long as it is specified in a valid LaTeX format, so that course of action is probably not helping anything. But let's discuss this in a separate issue report and keep this one focussed on the current changes.

If the test is still unreliable, so be it and let's inspect this one later on. I am in favor of removing svd to get rid of numerical problems. I will have a go at it tonight to verify the code here.

add comments

PeterPablo · 2015-04-13T08:47:36Z

Let's keep this PR's scope limited to SVD. width and hist() will be dealt with separately.

I restored the unreliable status of two tests, though added a comment about why this test is unreliable.
I removed the unreliable flag of bars (= ACID(34)). Please test, if hashes match for you!

If no further issues arise, I consider this ready.

PeterPablo · 2015-04-14T11:53:37Z

Please let's first get #590 in. I will then merge develop here and update the hashes. Afterwards somebody has to verify ACID(34) and this is then ready to go in.

see matlab2tikz#614

…e_numerical_issues Conflicts: test/suites/ACID.MATLAB.8.3.md5 test/suites/ACID.MATLAB.8.4.md5 test/suites/ACID.Octave.3.8.0.md5 test/suites/ACID.m

octave 3.8.2 R2014b R2014a from Xubuntu 12.04

see matlab2tikz#614

PeterPablo · 2015-04-15T15:06:55Z

I merged develop, updated the hashes and marked one more test unreliable and linked to the issue/discussion. Please test ACID(34) on Matlab R2014(a|b) and merge, as soon as Travis ran successfully.

reason for fail: different `width`; should be fixed/improved by matlab2tikz#604 diff: --- mine/test34-converted.tex +++ travis/test34-converted.tex @@ -1,3 +1,4 @@ +%%%% BEGIN FILE "data/converted/test34-converted.tex" %%%% % This file was created by matlab2tikz. % \documentclass[tikz]{standalone} @@ -18,9 +19,9 @@ \begin{tikzpicture} \begin{axis}[% -width=0.400089\figurewidth, -height=0.415744\figureheight, -at={(0\figurewidth,0.584256\figureheight)}, +width=0.410625\figurewidth, +height=0.418605\figureheight, +at={(0\figurewidth,0.581395\figureheight)}, scale only axis, point meta min= 0, point meta max= 1, @@ -222,9 +223,9 @@ \end{axis} \begin{axis}[% -width=0.400089\figurewidth, -height=0.415744\figureheight, -at={(0.584256\figurewidth,0.584256\figureheight)}, +width=0.410625\figurewidth, +height=0.418605\figureheight, +at={(0.540296\figurewidth,0.581395\figureheight)}, scale only axis, point meta min= 0, point meta max= 1, @@ -426,8 +427,8 @@ \end{axis} \begin{axis}[% -width=0.400089\figurewidth, -height=0.415744\figureheight, +width=0.410625\figurewidth, +height=0.418605\figureheight, at={(0\figurewidth,0\figureheight)}, scale only axis, point meta min= 0, @@ -540,9 +541,9 @@ \end{axis} \begin{axis}[% -width=0.400089\figurewidth, -height=0.415744\figureheight, -at={(0.584256\figurewidth,0\figureheight)}, +width=0.410625\figurewidth, +height=0.418605\figureheight, +at={(0.540296\figurewidth,0\figureheight)}, scale only axis, point meta min= 0, point meta max= 1, @@ -653,4 +654,6 @@ }; \end{axis} \end{tikzpicture}% -\end{document} +\end{document} + +%%%% END FILE "data/converted/test34-converted.tex" %%%%

…into remove_svd_reduce_numerical_issues

…ssues remove function calls to SVD

remove function calls to SVD

bce5680

* introduce sub function that returns a small data table * all svd calls are replaced * this should hopefully partially resolve issues, observed e.g. in matlab2tikz#590 and matlab2tikz#604

PeterPablo added the work-in-progress label Apr 10, 2015

PeterPablo added this to the Fix ACID Test milestone Apr 10, 2015

PeterPablo added 3 commits April 10, 2015 17:03

update R2014b hashes

1793287

update octave hashes

056d730

updatate HG1 hashes (R2011b on MAC used)

3fa8158

restore unreliable state of tests

9c53f53

add comments

PeterPablo removed the work-in-progress label Apr 13, 2015

This was referenced Apr 14, 2015

introduce font encoding #590

Merged

change float format #604

Merged

PeterPablo added the development label Apr 15, 2015

PeterPablo added a commit to PeterPablo/matlab2tikz that referenced this pull request Apr 15, 2015

mark stackedBarsWithOther unreliable

a2060a4

see matlab2tikz#614

PeterPablo added 3 commits April 15, 2015 17:03

Merge remote-tracking branch 'upstream/develop' into remove_svd_reduc…

f8e6839

…e_numerical_issues Conflicts: test/suites/ACID.MATLAB.8.3.md5 test/suites/ACID.MATLAB.8.4.md5 test/suites/ACID.Octave.3.8.0.md5 test/suites/ACID.m

update hashes

31bdad6

octave 3.8.2 R2014b R2014a from Xubuntu 12.04

mark stackedBarsWithOther unreliable

be35be0

see matlab2tikz#614

PeterPablo added 2 commits April 15, 2015 17:18

Merge branch 'develop' of https://github.com/matlab2tikz/matlab2tikz …

874447e

…into remove_svd_reduce_numerical_issues

okomarov pushed a commit that referenced this pull request Apr 15, 2015

Merge pull request #614 from PeterPablo/remove_svd_reduce_numerical_i…

2a8c33b

…ssues remove function calls to SVD

okomarov merged commit 2a8c33b into matlab2tikz:develop Apr 15, 2015

PeterPablo deleted the remove_svd_reduce_numerical_issues branch April 16, 2015 09:28

egeerardyn mentioned this pull request Jun 17, 2015

Tackle all TODOs and FIXMEs in the code #617

Open

97 tasks

remove function calls to SVD #614

remove function calls to SVD #614

PeterPablo commented Apr 10, 2015

PeterPablo commented Apr 10, 2015

PeterPablo commented Apr 10, 2015

okomarov commented Apr 11, 2015

PeterPablo commented Apr 11, 2015

okomarov commented Apr 11, 2015

PeterPablo commented Apr 11, 2015

PeterPablo commented Apr 11, 2015

egeerardyn commented Apr 13, 2015

PeterPablo commented Apr 13, 2015

PeterPablo commented Apr 14, 2015

PeterPablo commented Apr 15, 2015

remove function calls to SVD #614

remove function calls to SVD #614

Conversation

PeterPablo commented Apr 10, 2015

PeterPablo commented Apr 10, 2015

PeterPablo commented Apr 10, 2015

okomarov commented Apr 11, 2015

PeterPablo commented Apr 11, 2015

okomarov commented Apr 11, 2015

PeterPablo commented Apr 11, 2015

PeterPablo commented Apr 11, 2015

egeerardyn commented Apr 13, 2015

PeterPablo commented Apr 13, 2015

PeterPablo commented Apr 14, 2015

PeterPablo commented Apr 15, 2015