Skip to content

Fix truncated qhull stdio - #187

Open
papillot wants to merge 2 commits into
Discngine:masterfrom
papillot:fix-truncated-qhull-stdio
Open

papillot wants to merge 2 commits into
Discngine:masterfrom
papillot:fix-truncated-qhull-stdio

Conversation

@papillot

@papillot papillot commented Oct 7, 2026

Copy link
Copy Markdown

This PR fixes a bug where the output from qvoronoi is read before it is flushed: the list of vertices is truncated. Depending on the size of the truncation, this can lead to an entire pocket missed, or only some vertices in a pocket (which still impacts the computed predictors).

Measured on Linux x86_64 (Ubuntu 24.04, gcc 13) with an instrumented build of master counting the vertices fill_vvertices() actually reads:

Case Vertices from qhull Read on master Lost Pockets master Pockets with fix
1UYD 10703 10604 99 13 13
3LKF 15308 15099 209 18 18
1ATP 19455 19445 10 20 20
4URL 36365 36189 176 39 40
5WA6 9113 9029 84 12 12
7TAA 24453 24310 143 24 24
2P0R -c D 34259 34146 113 43 43
1UYD -r 1224:PU8:A 10703 10604 99 1 1
2P0R_mod -a D 34259 34146 113 1 1
3VI4 151157 151131 26 240 240
5RGF 30254 30122 132 24 24
6TL9 55858 55689 169 98 98
6X3P -w both 14374 14197 177 16 16
1QNH -a C,D 17011 16891 120 1 1

PR is split in 2 commits: the code change itself vs the test files changes which are larger (e.g. shifts due to changes in pocket ranking).

Note: This bug was found by comparing results from a WASM instance of fpocket vs native. Due to differences in the stdio buffer length, inconsistencies were found.

Note 2: This investigation and PR had been made using a coding agent

papillot and others added 2 commits October 7, 2026 11:35
load_vvertices() reopens the qvoronoi output file for reading while ftmp,
the stream qhull wrote it through, is still open and unflushed. The reader
only sees what stdio has already written to disk, so the end of the file
(neighbour lists of the last vertices) is silently dropped. How many
vertices are lost depends on the libc buffer size and the output size:
on Linux/glibc the sample structures in data/sample lose 10 to 209
vertices each, and results differ between platforms.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Regenerated on Linux x86_64 (Ubuntu 24.04, gcc 13) with the fixed binary.
Only files the tests compare as different are replaced; the others are
unchanged. 4URL gains a pocket (40), and pocket renumbering explains the
large number of files for 6TL9 and 123abc.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant