Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move use fpu reciprocal to the fpu library #861

Closed
ghost opened this issue May 24, 2020 · 21 comments
Closed

Move use fpu reciprocal to the fpu library #861

ghost opened this issue May 24, 2020 · 21 comments

Comments

@ghost
Copy link

ghost commented May 24, 2020

First of all, sorry, this is to a large extent my bad. When writing the update of the bbox library, I added the use fpu reciprocal key. At this point I had sort of forgotten that this key gets also used in CircuitikZ. The definitions are the same (in the sense that the functionality is the same). The key is defined via

\pgfqkeys{/pgf}{use fpu reciprocal/.code={%
\def\pgfmathreciprocal@##1{%
    \begingroup
    \pgfkeys{/pgf/fpu=true,/pgf/fpu/output format=fixed}%
    \pgfmathparse{1/##1}%
    \pgfmath@smuggleone\pgfmathresult
    \endgroup
}}}%

I wonder if we can move it to the fpu library.

P.S. According to my experience this key can be very useful beyond CircuitikZ and bbox. It does cure a large fraction of dimension too large errors that occur in decorations.

@ghost ghost added the Feature Request label May 24, 2020
@Rmano
Copy link

Rmano commented May 24, 2020

This key fix a precision problem --- you can see it in circuitikz/circuitikz#367 --- when big coordinates are used.

For a test where circuitikz is not involved, you can see here: https://tex.stackexchange.com/questions/529099/path-joining-in-to-path-with-non-integer-scale-factors/

@ghost
Copy link
Author

ghost commented May 24, 2020

One example where this key removes the dimension too large errors is https://tex.stackexchange.com/a/540276/194703. There are many more such examples.

Rmano added a commit to Rmano/circuitikz that referenced this issue May 24, 2020
@Mo-Gul Mo-Gul added the fpu label May 24, 2020
@hmenke
Copy link
Member

hmenke commented May 24, 2020

@Rmano No, it is not a precision problem, but a problem with the range that can be represented. In fact, precision is much worse when using the FPU. For example:

\documentclass{article}
\usepackage{tikz}
\usetikzlibrary{fpu}
\begin{document}
\pgfmathparse{12/4}\pgfmathresult

\tikzset{/pgf/fpu=true,/pgf/fpu/output format=fixed}
\pgfmathparse{12/4}\pgfmathresult
\end{document}

@tallmarmot I hope this definition you posted is a joke and you are not actually using this.

@ghost
Copy link
Author

ghost commented May 24, 2020

@hmenke I am absolutely using it, it is absolutely not a joke, and you should not use examples where the use of fpu does not make sense. You post an example integer over integer and draw your conclusions from one example. This is not the important case when the built in reciprocal sucks. Try

\documentclass{article}
\usepackage{tikz}
\usetikzlibrary{fpu}
\begin{document}
\pgfmathparse{12.34/0.001234}\pgfmathresult

\tikzset{/pgf/fpu=true,/pgf/fpu/output format=fixed}
\pgfmathparse{12.34/0.001234}\pgfmathresult
\end{document}

There is a reason why @Rmano and I are using it. While the discrepancy for integer divisions does not have any visible impact on paths, the difference for non-integer fractions is HUGE and can be seen in https://tex.stackexchange.com/questions/529099/path-joining-in-to-path-with-non-integer-scale-factors/ as well as all the cases in which you get these very unnecessary dimension too large errors.

@hmenke
Copy link
Member

hmenke commented May 24, 2020

Aha, I see. And instead of improving the built-in code we must load 3000 lines of library code that has the capability of breaking basic drawing. No, thanks.

@ghost
Copy link
Author

ghost commented May 24, 2020

@hmenke That's why you can switch it on and off. (I actually do not see how the changing the reciprocal can break drawings, but as I said the change is local.)

Here is a code that allows you to see what happens. It assumes that xfp is sort of close to the true result, which is an assumption that I cannot prove.

\documentclass{article}
\usepackage[margin=1cm]{geometry}
\usepackage{xfp}
\usepackage{etoolbox}
\usepackage{longtable}
\usepackage{tikz}
\usetikzlibrary{fpu}
\newcommand{\pgfmathsetmacroFPU}[2]{\begingroup
\pgfkeys{/pgf/fpu=true,/pgf/fpu/output format=fixed}%
\pgfmathsetmacro{#1}{#2}%
\pgfmathsmuggle#1\endgroup}
\begin{document}
\pgfmathsetseed{\number\pdfrandomseed}%
\edef\iloop{0}%
\edef\maxerrorplain{0}*
\edef\maxerrorfpu{0}*
\loop
\pgfmathsetmacro{\myx}{pow(0.1*(rnd+0.1),2)}%
\pgfmathsetmacro{\myy}{pow(0.1*(rnd+0.4),3)}%
\pgfmathtruncatemacro{\itest}{\myy>0.001}%
\ifnum\itest=1
\pgfmathsetmacro{\plainresult}{\myx/\myy}%
\pgfmathsetmacroFPU{\fpuresult}{\myx/\myy}%
\edef\xfpresult{\fpeval{\myx/\myy}}%
\edef\errorplain{\fpeval{(\plainresult-\xfpresult)/\xfpresult}}%
\edef\errorfpu{\fpeval{(\fpuresult-\xfpresult)/\xfpresult}}%
\pgfmathsetmacro{\maxerrorplain}{max(abs(\errorplain),\maxerrorplain)}%
\pgfmathsetmacro{\maxerrorfpu}{max(abs(\errorfpu),\maxerrorfpu)}%
\xappto\mytablecontents{$\myx$ & $\myy$ & $\plainresult$ & $\fpuresult$
 & $\xfpresult$ & $\errorplain$ &  $\errorfpu$}
\gappto\mytablecontents{\\}
\edef\iloop{\the\numexpr\iloop+1}%
\fi
\ifnum\iloop<500\repeat

max error plain: $\maxerrorplain$, max error fpu: $\maxerrorfpu$\par\bigskip


\begin{longtable}{lllllll}
$x$ & $y$ & plain & fpu & xfp & error plain & error fpu\\
\endhead
\endlastfoot
\mytablecontents
\end{longtable}
\end{document}

The upshot of my run with this document is

max error plain: 0.06804, max error fpu: 0.00005

So you can easily have errors larger than 5% with the built in reciprocal but at least in this comparison the fpu errors are well below the permille level. Let me stress that I did not design this snippet to maximize the error of the plain code. In fact, I had to filter out the cases where it is particularly bad, which is when it is close to dimension too large errors.

It is IMHO evident that fpu does NOT generally decrease the precision. In all but exceptional cases it makes the results more precise. It does certainly help tremendously with avoiding dimension too large errors in decorations and the bounding box. In general, it helps when inverting transformations that go beyond integers.

If you want to tell the users that you do not want allow them to locally switch on the fpu reciprocal, this is your choice. I would not want to make this choice personally.

How can we proceed in a constructive way? The only real alternatives I see is to rewrite the built in reciprocal, but this is something I personally would want to try out, or to load xfp if you really think that fpu is so bad at reciprocals. (We all know that it isn't great at computing trigonometric functions for small arguments. But this is not part of this discussion.) So what do you suggest?

@hmenke
Copy link
Member

hmenke commented May 25, 2020

I think that the fpu is bad at everything and is really a potential footgun. Instead of adding super-secret obscure extra keys to work around random corner cases here and there, it is much more reasonable to try to design something that works reliably out-of-the-box for all users.

Also please don't post huge code blocks. Use a GitHub Gist or some other paste service for that.

@ghost
Copy link
Author

ghost commented May 25, 2020

@hmenke You started out with the statement that fpu decreases the precision. Unfortunately, that is not correct. I already noted that you do not like fpu. That's your choice. But please do not imply that what I am suggesting does not work reliably. It does. And no, I am not going to spend more time to come up with another reciprocal because I know what will happen. Even if my new reciprocal works perfectly, you will say something negative about it without even really looking at it. This is all your choice. I have tried to add something that helps many users in many situations in a sustainable way, and for which there is not evidence of any downside whatsoever. You do not like it and decide to dismiss it. Fine, this terminates the discussion, and my motivation to spend time on it.

@hmenke
Copy link
Member

hmenke commented May 25, 2020

I don't mean to dismiss these great ideas, but you have to be aware that including this is a massive regression. It replaces one class of errors (Dimension too large), that users are already used to and know how to circumvent, by another one (loss of precision for integer division) which is completely new and unexpected for users.

The reason why I don't like the fpu is the fact that it is not actually a FPU, which leads to unpleasant surprises and disappointment (at least on my side). The code is also not maintainable because computation and representation are tightly interwoven, which is why #678 is basically unfixable short of rewriting the entire fpu library.

@ghost
Copy link
Author

ghost commented May 25, 2020

@hmenke The error of the traditional reciprocal can be larger than 13%.

\documentclass{article}
\usepackage{pgf}
\usepackage{xfp}
\begin{document}
\edef\myx{0.000069}
\pgfmathparse{1/\myx}\pgfmathresult

\fpeval{1/\myx}

\fpeval{(\pgfmathresult-\fpeval{1/\myx})/\fpeval{1/\myx}}
\end{document}

The problem is the code/case

\c@pgfmath@counta#1#2#3#4#5#6\relax%
\c@pgfmath@countb1000000000\relax%
\divide\c@pgfmath@countb\c@pgfmath@counta%
\c@pgfmath@counta\c@pgfmath@countb%
\divide\c@pgfmath@counta10000\relax%
\pgfmath@x\c@pgfmath@counta pt\relax%
\multiply\c@pgfmath@counta-10000\relax%
\advance\c@pgfmath@countb\c@pgfmath@counta%
\pgfmath@y\c@pgfmath@countb pt\relax%
\pgfmath@y.1\pgfmath@y% Yes! This way is more accurate. Go figure...
\pgfmath@y.1\pgfmath@y%
\pgfmath@y.1\pgfmath@y%
\pgfmath@y.1\pgfmath@y%
\advance\pgfmath@x\pgfmath@y%

Here #1 is what is before the period and #2#3#4#5#6 the digits after that. In the above case they are #2#3#4#5#6=00006. The problem is that there is no check whether there are leading zeros, i.e. #2=0 or #2=#3=0 or #2=#3=#4=0 or, as in this case #2=#3=#4=#5=0. So, if you really want to improve the core function, one has to build in these checks. This would amount to see what #7 is and extract the digits accordingly. Certainly doable. The downside is that if there is a mistake, users won't have a chance to just switch this thingy off.

@ghost
Copy link
Author

ghost commented May 25, 2020

@hmenke Final comment on issue 678. Switching on fpu when one draws paths is in my opinion stupid. But this is not at all what this proposal is about. It replaces a select function by an fpu variant. When TikZ does its normal operations, fpu is switched off. These are really two conceptually totally different things. I have done the "switch on fpu locally" a lot and never gotten any problem. So the issue 678 is completely irrelevant for the present discussion.

@hmenke
Copy link
Member

hmenke commented May 25, 2020

The problem occurs much earlier before even entering reciprocal. On entry to divide the numbers are converted into dimen register and that is where you lose all that precision.

\pgfmath@x=#1pt\relax%
\pgfmath@y=#2pt\relax%

The key issue with use fpu reciprocal is that 90% of users (probably even more) don't ever read the manual. They just copy your answers from TeX.SX which might contain use fpu reciprocal. They don't know what that is but it fixes their problem, so it gets used and abused and it starts raining bug reports over here.

@Mo-Gul
Copy link
Contributor

Mo-Gul commented May 25, 2020

Maybe this is not the right place add my thoughts as well, but I'll give it a try and you -- of course -- decide what to do with it. Please also note that your math knowledge and understanding is way higher than mine, so I might be completely wrong.

My feeling about the math precision stuff is that it would be best to add another option (or two) as well.

  • So one could use Lua for all the math stuff if the document is compiled with LuaTeX and friends. This should give both: A result with a very high precision as well as a very low computation time.
  • Alternatively one could use the already above mentioned xfp package as calculation engine which should give a good trade-off between precision and computation time.

Here I link two TUGBoat articles of Joseph Wright that deal with the above.

Of course I don't have any clue how much work that would imply, but hopefully it will be worth it. What I know is that PGFPlots already did something to use Lua as calculation engine in some circumstances (see section 6.3.1 in the PGFPlots manual (v1.17)). Maybe this helps!?

@ghost
Copy link
Author

ghost commented May 25, 2020

@hmenke So far there is no issue with the key, and I seriously doubt it will be raining bug messages. If anything, this key will decrease the number because it can cure the problems of decorations. I am also not sure if the conversion to dimensions is the main effect. At least I understand the huge error of over 13% more or less entirely from the code of the reciprocal. If you want to clean up on the Q & A sites, tell people even more strongly not to nest tikzpictures nor to use \pgfextra to construct new paths.

(I had the last discussion of this type just yesterday, see here. The answer is still unchanged, and if you add rotate=20, say, to his \path command everything goes berserk. The answer stays, gets copied, and this is what causes the problems, similarly to nesting tikzpictures. So really, do not blame me for what I am doing, I am very sure that the local use of use fpu reciprocal is fine.)

@ghost
Copy link
Author

ghost commented May 25, 2020

@Mo-Gul I am very sure that adding any of these will be absolutely nontrivial. It would amount to rewrite a huge amount of code. IMHO LuaLaTeX is not an option, the arXiv does not support it. This would give TikZ the status of PSTricks: you cannot really use it for arXiv submissions. As for xfp: this might be possible but enormous efforts. Plus there are many things it does not do. Apart from the lack of a documentation it does not check if the input has units. So the complete logic of coordinate computations (radius vs. radius factor) would be destroyed. What could work is to replace the reciprocal, say, by an xfp variant. Honestly I do not see an advantage over fpu for that one.

@hmenke
Copy link
Member

hmenke commented May 25, 2020

@Mo-Gul xfp is on my list. Lua math is broken, see #699

@Rmano
Copy link

Rmano commented May 25, 2020

I am probably mathematically less knowledgable than @Mo-Gul here (I am just an electronic engineer, and an analog one --- we're in process of extinction) but I am working in instrumentation so I am quite used to errors.

I think that the 12/4=2.9999000 has the problem that it is using too many digits --- the calculation is done mantissa+exponent with 5 significant digits, for what I can understand, so the result is 2.9999 which in this representation is the same as 3 (it happens with binary floating points... I have a lot of students asking why 1/10 is not 0.1...), so practically there is no error (in this domain).

When using fixed arithmetic, the range is a problem, and there is nothing you can do for the loss of precision for very small numbers --- short of switching to mantissa+exponent, which has the representation limits problems (this is why for example in Python you have a Decimal class...). So I reckon it's a hard problem. The Universe is mean.

I agree that a real solution should be to change the internal math to have a more correct value (and all my kudos for trying to integrate xfp), and that what pgf is doing, giving the limitations of TeX arithmetic, is almost magical. But the use fpu reciprocal thing was not supposed to be a default thing --- just a stopgap solution for problems that sometimes arise. The issue I have linked in circuitikz was a real one...

But again, it's not so fundamental. I agree that opinions can differ, and I agree that sometime stopgap solutions can be bad on long time scales. My opinion is that this one, used sparingly, can help.

@ghost
Copy link
Author

ghost commented May 25, 2020

To summarize: @hmenke made three completely false statement: precision of fpu is much worse: wrong, relation to issue 678: wrong, "numbers are converted into dimen register and that is where you lose all that precision": wrong, and used strong language to go after others ("I hope this is a joke"). Do whatever you like but I am not willing to continue this.

hmenke added a commit to hmenke/pgf that referenced this issue May 26, 2020
This key will selectively install the specified functions from the fpu
library to be able to avoid `Dimension too large' errors in specific
cases.  Be aware that under certain circumstances the fpu comes with a
loss of precision due to the underlying fixed-point arithmetic of TeX.
These situations are rare but have be looked out for.

[[ Example: In general the fpu increases the range of permissible number
            substantially but sometimes has precision problems.

    \usetikzlibrary{fpu}
    \pgfkeys{/pgf/fpu/install only={divide}}
    \pgfmathparse{12.34/0.001234}\pgfmathresult % 10000.000000 (good)
    \pgfmathparse{12/4}\pgfmathresult % 2.9999000000000 (bad)

]]
@hmenke
Copy link
Member

hmenke commented May 26, 2020

How about this? 494bd67 (Docs 1612b81)

This doesn't come with the massive overhead of reparsing the expression, even though at this point we already know it's a number and it is generic, i.e. you can temporarily install any function from the FPU. Currently this issues a warning because I have not tested this thoroughly and I'm not so happy with the implementation, so I might change/rename/remove this again.

@Rmano
Copy link

Rmano commented May 26, 2020

I tried it with the issue in circuitikz:

https://gist.github.com/Rmano/e7c48d09be4cea71ea5e7d3b20f9808b

(I hope to have copied all the relevant bits).

It works for install only={reciprocal}, but if I try install only={divide} it fails with a lot of dimension expected things --- which is maybe expected for the comment "Unfortunately, the FPU is currently incompatible with drawing operations." (sorry, I am not really a low-level TeX guy, so I hope I am not saying silly things... please bear with me ;-) ).

@hmenke
Copy link
Member

hmenke commented May 26, 2020

Yes, I can confirm that installing divide breaks pretty much all drawing code. That actually makes this a nice debugging tool. Maybe it's possible to make the FPU work with drawing code, by selectively testing different functions.

@hmenke hmenke closed this as completed May 28, 2020
Rmano added a commit to Rmano/circuitikz that referenced this issue May 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants