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

floating poind rounding error leads to assertion failure #302

Open
hartmans opened this issue Oct 30, 2021 · 2 comments
Open

floating poind rounding error leads to assertion failure #302

hartmans opened this issue Oct 30, 2021 · 2 comments
Labels
bug crash rinohtype aborts due to an uncaught exception

Comments

@hartmans
Copy link

On master commit ed1b2b2
I get a crash with an assertion failure at line 1112 in paragraph.py, the assertion failes calling container.advance2(descender)
The problem is that the container has just enough height left, but there's a floating point rounding error:

(Pdb) p float(container.remaining_height) + descender
-5.329070518200751e-15

I'd prefer not to share the source files, although we could talk about doing so privately if it's really necessary.
However, whenever there's a chance of a floating point comparison needing exact equality, you need to account for rounding error like the above.
I think the appropriate fix would either be in advance2 or in the equality operators of the dimension objects depending on how global of a fix you want.

@hartmans hartmans added bug crash rinohtype aborts due to an uncaught exception labels Oct 30, 2021
@hartmans
Copy link
Author

diff --git a/src/rinoh/layout.py b/src/rinoh/layout.py
index 48b7745b..ff6160a8 100644
--- a/src/rinoh/layout.py
+++ b/src/rinoh/layout.py
@@ -271,7 +271,7 @@ class FlowablesContainerBase(Container):
         bottom of the container.
 
         """
-        if height <= self.remaining_height:
+        if height <= self.remaining_height+1e-10:
             self._self_cursor.grow(height)
         elif ignore_overflow:
             self._self_cursor.grow(float(self.remaining_height))

is a quick fix for what I'm running into.
I'm not going to go fill out a CLA and submit that as a pull request because it's too sloppy. I'd assume at a minimum you'd want to define some global fuzz constant (possibly even make it configurable) because it seems likely you'll run into this sort of issue in other places as well.

@brechtm
Copy link
Owner

brechtm commented Nov 4, 2021

Thanks for the bug report and analysis!

Floating point rounding errors are in fact something I wondered about before, but I haven't yet run into any issues because of them myself. A proper solution would be to use Python's Decimal type. These are reportedly 3 times slower than floats, but I doubt that would have much of an impact on the total rendering time. Alternatively, integers representing 1/100th of a Postscript point could be used.

I've committed your suggested quick fix, but I'll leave this issue open to track the bigger issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug crash rinohtype aborts due to an uncaught exception
Projects
None yet
Development

No branches or pull requests

2 participants