Repository navigation
Conversation
ukaratay
added this pull request to stack #7
October 4, 2026 18:19
rnewman
approved these changes
Oct 6, 2026
parse_test.rs and config_test.rs defaulted to expedia/nt50_md8, a cell no local prepare run produces. Two parse tests failed on every branch because of it: test_reject_non_json panicked on the missing walker_config.json instead of the extension check, and test_reject_unknown_json_schema returned early from a should_panic test. The other four returned before testing anything. parse_test.rs now defaults to flchain/nt500_md8_h16, whose LightGBM JSON export is valid, so all six tests run. config_test.rs asserts Expedia's grouping and defaults to expedia/nt500_md8, whose walker_config.json has the same 21 features, 10 varying features and 37-row maximum. The two reject tests no longer need artifacts: they write a five-field configuration, and the bad model that used to go to /tmp/bad_model.json, under CARGO_TARGET_TMPDIR. Treelite writes `"threshold": ,` for nonfinite values, as in the Expedia LightGBM export (22 times). The loader rejects such dumps instead of repairing them, as docs/treelite-loading.md documents. The four parse tests that load a LightGBM JSON export and correctness.rs's test_binary_matches_json skip a file only when its load fails with a JSON syntax error and a byte scan, independent of the parser under test, finds empty thresholds in it; they print the path, the count and the error. Any other load error fails, so a parser regression on valid JSON cannot hide as a skip, and every other JSON export, including Expedia's XGBoost one, is still compared with its binary. Skips for missing models or predictions in the correctness suite are printed too. Co-authored-by: AI (Pi/Claude Opus 5.5) <noreply@pi.dev>
rust-toolchain.toml pins 1.97.1, but neither package declared a rust-version, so Cargo could resolve dependencies that need a newer compiler and crates.io would publish treewalker-gbdt without a minimum. The version is set once in [workspace.package]; that table alone applies nothing, so both packages inherit it with rust-version.workspace = true. Resolver 3, the edition 2024 default, is set explicitly: it is the resolver that picks dependency versions compatible with rust-version, which the dependency upgrades that follow rely on. Co-authored-by: AI (Pi/Claude Opus 5.5) <noreply@pi.dev>
simd-json 0.14 was four minor releases behind. Before upgrading, serde_json 1.0.151 was measured against simd-json 0.18.1 on the Expedia XGBoost export (float32, 52,979,912 bytes) and the FLCHAIN horizon-1,024 LightGBM export (float64, 65,694,653 bytes, the largest valid one), each through a copy of the loader ported to the other parser. On the M4 Pro, median of 7: - load time: serde_json 153 and 199 ms, simd-json 112 and 148 ms (1.35x); - peak RSS: serde_json 502 and 563 MB, simd-json 528 and 659 MB; - parsed values: by default serde_json misparses 2,258 of 84,692 thresholds and 8,938 of 85,192 leaves on Expedia and 28,382 of 215,186 FLCHAIN node values against Rust's correctly rounded parse. With float_roundtrip it matches, as do simd-json 0.14 and 0.18, and every node pool equals the binary checkpoint's bit for bit; - errors: serde_json accepts integers beyond 64 bits as floats and rejects lone surrogates, which simd-json does the other way around, and keeps the last of duplicate keys where simd-json keeps the first. The 64 MiB size limit and the depth-64 check run before either parser. serde_json is within 1.5x but does not match on values without a feature or on error behavior, so the loader stays on simd-json. 0.18 needs no source changes, parses every value identically to 0.14, and loads in the same time. Co-authored-by: AI (Pi/Claude Opus 5.5) <noreply@pi.dev>
libloading 0.9 moved the system's dlerror message from the error's Display text into its source, so the harness's load messages would have said only "dlopen failed" or "dlsym failed". A small report helper prints the source after the error, as before: for example "lleaves: symbol error: dlsym failed: dlsym(..., forest_root): symbol not found". Symbol names are now C string literals, which 0.9 accepts without the null-byte check and copy that byte strings need. Co-authored-by: AI (Pi/Claude Opus 5.5) <noreply@pi.dev>
cargo update, resolved against rust-version 1.97, moves every other crate to its latest compatible release: among the direct dependencies, libc 0.2.190, serde 1.0.229 and rustc-hash 2.1.3; the rest are transitive, mostly through ndarray-npy and the QuickScorer baseline. No crate is held back by the minimum Rust version. ndarray 0.17 and ndarray-npy 0.10 are already the latest minor releases. Co-authored-by: AI (Pi/Claude Opus 5.5) <noreply@pi.dev>
cargo doc --no-deps warned that the crate docs link to CellData and collect_methods, which are private, so the links could not resolve. They are plain code spans now, and cargo doc builds both packages without warnings under every feature combination. Co-authored-by: AI (Pi/Claude Opus 5.5) <noreply@pi.dev>
rnewman
force-pushed
the
dkaratay/v2-deps
branch
from
October 6, 2026 23:16
356cbc3 to
a3d2c7f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every Rust dependency moves to its latest release that builds with Rust 1.97, and both packages now declare that minimum. The library's source does not change.
The JSON loader stays on simd-json, now 0.18.1. Before upgrading I measured serde_json 1.0.151 against it, through a copy of the loader ported to each parser, on the Expedia XGBoost export (float32, 53 MB) and the largest valid export, FLCHAIN horizon 1,024 LightGBM (float64, 66 MB). M4 Pro, 2026-10-04, median of 7:
serde_json is within the 1.5× load time we set as the limit for switching, but with default features it misparses floats: 28,382 of FLCHAIN's 215,186 node values differ from Rust's correctly rounded parse. Its
float_roundtripfeature fixes that. It also handles 4 of 29 malformed inputs differently: it accepts integers wider than 64 bits, rejects lone surrogates, and keeps the last of duplicate keys. simd-json 0.14 and 0.18 parse every value identically, and both node pools equal the binary checkpoints bit for bit.rust-version = "1.97"is set once in[workspace.package]and inherited by both packages. Resolver 3 is explicit; it is the resolver that picks versions compatible with that minimum.cargo updateheld nothing back.dlerrortext from the error'sDisplayinto itssource(). The harness prints the source, so a missing symbol still reads "dlsym failed: dlsym(..., forest_root): symbol not found".Tests:
parse_testandconfig_testdefaulted toexpedia/nt50_md8, a cell no prepare run produces. Two parse tests failed on every branch, and the other four returned without testing anything.parse_testnow defaults toflchain/nt500_md8_h16, where all six run, andconfig_testtoexpedia/nt500_md8. The two reject tests need no artifacts and no longer write to/tmp.test_binary_matches_jsonfrom Exact leaf sums and groups of any width #6, skip an export only when a byte scan finds Treelite's empty"threshold": ,values, 22 of them in the Expedia LightGBM dump. Any other parse error fails, so a parser regression cannot hide behind the skip. 15 exports are compared and 1 is skipped.Production
predict()before and after, alternating builds over 5 rounds (M4 Pro, 2026-10-04), is within ±4% in 12 of 14 cells, from G = 16 to 1,008 plus Expedia. G = 256 is 11–12% slower in the default build, but its predict functions are instruction-for-instruction identical; only their addresses moved. Rebuilt with 64-byte function alignment, every cell is within ±3%, except FLCHAIN horizon 1,024 XGBoost, which is 6% faster. From this PR on, a slowdown counts as a regression only if it persists under fixed function alignment.