Conversation
Split monolithic main.rs into parser.rs, tree.rs, lca.rs, midpoint.rs.
Critical fixes:
- Rewrite midpoint_root to fix infinite loop from cyclic graph
- Add NHX/bracket comment parsing to prevent crash on annotated trees
- Add single-quoted label support in parser
- Detect and reject duplicate leaf names
Additional fixes:
- Add --precision flag for floating point output formatting
- Add -t/--threads flag for rayon thread control
- Add stdin support (use '-' as phylogeny argument)
- Warn on missing branch lengths in patristic mode
- Warn on negative branch lengths
- Always print leading tab in header for R/Python compatibility
- Use env!("CARGO_PKG_VERSION") for version string
- Remove unused --format flag
- Fix clippy warnings (&Vec<Node> -> &[Node])
- Add 18 unit tests
- Rewrite parser to iterative (no recursion): handles 50k+ deep trees - Rewrite flatten_raw to iterative: prevents stack overflow on flatten - Strip whitespace/newlines outside quotes before parsing - Support double-quoted labels (common in BEAST, FigTree output) - Warn on --lmm + --topology conflict instead of silent precedence - Topology mode outputs integers instead of float decimals - Pre-process Newick string to remove whitespace outside quotes - 24 tests (6 new: whitespace, newlines, deep tree, double quotes, nested brackets, negative length parsing)
- Replace process::exit with Result<(), Box<dyn Error>> for clean error propagation - Add --lower flag for PHYLIP-compatible lower triangle output - Add DistMode enum to centralize mode logic - Add compute_distance() and format_distance() helpers - Add CI workflow (clippy + test on push/PR) - Add Build & Release workflow (4 binaries on tag push) - BufWriter on stdout for better performance - Better error messages (file not found, parse errors) - 28 tests (4 new: single leaf, symmetric tree, self-distance, mode conflict) - Update README with new flags and features - 0 clippy warnings with -D warnings
…, doc comments, LCA tests, row buffer reuse
…ADME troubleshooting fix
Newick from IQ-TREE, MrBayes and BEAST often starts with a rooting marker such as '[&R] (...)', and BEAST also puts per-branch metadata before the label rather than after it. Comments were only skipped once a label had been read, so '[&R] (A,B);' parsed as one unnamed leaf and died with "No labeled leaves found in the tree", while '(A:1,[&x=1]B:2);' failed with a confusing "Expected ',' or ')'". Skip whitespace and bracketed comments at the start of every subtree.
Labels were assembled with 'byte as char', which turns every byte of a multi-byte UTF-8 sequence into a code point of its own. A tree with 'Senor_Nu' spelled with its accents came back out of the matrix as mojibake, so any label with an accent, a Greek letter or CJK text silently stopped matching the sample names used downstream. Collect the label bytes and decode them as UTF-8 in one step. Scanning byte-wise stays valid because every Newick delimiter is ASCII.
Several kinds of broken input were accepted without a word and turned
into a plausible looking but wrong matrix:
- A truncated tree ('((A:1,B:2),C:3' with the last ')' missing) had its
open parentheses closed at end of input, so every internal node left
dangling silently lost its branch length.
- Text after the tree was dropped, so a file holding several trees
quietly produced a matrix for the first one only.
- An unclosed '[' swallowed the rest of the file as comment text.
- '(' was not a label delimiter, so prose such as 'not a tree at all
(((' parsed as a single leaf named after it.
Each of these now fails with a message naming the offending position.
Any apostrophe anywhere in the file opened a quoted region for the
whitespace-stripping pass, so "O'Brien:1.0 , B:2.0" left the rest of the
tree looking quoted and failed with "Expected ',' or ')', found ' '". An
apostrophe inside a comment did the same.
A quote now opens a label only at the start of the tree or right after
'(', ',' or ')'. The pass also consumes a doubled quote as the escaped
literal it is, rather than closing and immediately reopening the region,
which used to strip the whitespace in "'it''s a name'".
The parser and the flattener were rewritten iteratively in 1.0.1 so that deep trees would work, but nothing was done about RawNode's drop glue, which still recurses once per level. A 300,000-level ladder therefore still aborted with "has overflowed its stack", after parsing had already succeeded, because the parse tree was freed on the way out. Dismantle the children iteratively in a hand-written Drop.
'-o results.tsv' opened and truncated the file before the tree was even read, so a run that then failed on a malformed tree left the user with an empty file where their previous matrix used to be. Open the output once parsing, midpoint rooting and the leaf-label checks have all succeeded.
Nothing ever flushed the writer explicitly. BufWriter does flush when it is dropped, but it has nowhere to return an error and throws it away, so a full disk left a silently truncated matrix behind and an exit code telling the caller everything went fine. Flush explicitly and fail with the output path in the message. The opposite case is 'distree tree.nwk | head', which hit EPIPE partway through and printed "Error: Broken pipe (os error 32)" for what is a perfectly normal way to stop reading. That one now exits clean.
'-p 50000000' reached the formatting machinery unchecked and aborted with "Formatting argument out of range", a panic and a backtrace note for what is simply a bad argument. Reject anything above 30 decimals, which is already well past what a 64-bit float can distinguish. '-t 0' was passed straight to Rayon, which reads 0 as "choose the default" and started a thread per core, the opposite of what the flag was asked for, and no way to tell.
Tab characters in a label were already rejected because they would split a row, but a quoted label may hold any whitespace, and a newline or a carriage return breaks the output exactly the same way. A label with a newline in it produced a matrix whose row count no longer matched its header. Unlabeled leaves were a quieter version of the same surprise. They cannot be named in the matrix so they are skipped, and a tree written with empty labels came back one row shorter with nothing said about it.
Midpoint rooting inserts a node halfway along the diameter edge, which splits that edge in two and adds one hop to every pair whose path crosses it. On '((A:1,B:1):1,(C:1,D:1):1);' the A-C distance went from 4 to 5 the moment --midpoint was added. Edge counts do not depend on where a tree is rooted, so there is nothing for --midpoint to contribute in topology mode. Skip the rooting there and say why, rather than reporting counts that are one too high.
Midpoint rooting rewires parent/child links and swaps branch lengths all the way up to the old root, which the three hand-written trees exercised only lightly. Cross-check it on 300 generated trees, with polytomies, zero length branches and 2 to 25 leaves, against the three properties that matter: every pairwise patristic distance survives the rewiring, the result is still a tree with no cycle and nothing orphaned, and the deepest tip sits exactly half a diameter from the new root.
The binary-lifting table is the largest allocation distree makes, n log n entries, and each one was an Option<usize>. That type has no spare niche, so every entry paid 8 bytes for the ancestor and 8 more for a discriminant that only ever marks the root. Measured on a 4,000-leaf tree, peak RSS drops from 7.2 MB to 6.4 MB, which is the 0.8 MB the table itself was wasting. The same arithmetic on a million-leaf tree comes to about 350 MB. Store the ancestors as plain usize with usize::MAX standing in for the root, in one flat allocation rather than a Vec per level. Also cross-check mrca() against walking up from both nodes, over every pair in 200 generated trees, since the query loop was rewritten with it.
The path was hardcoded to target/debug/distree, so 'cargo test --release' ran whatever stale debug binary happened to be lying around and reported a pass for code it never executed. On a clean checkout it could not spawn anything at all. CARGO_BIN_EXE_distree resolves to the profile under test.
The clamp to zero was there to absorb floating-point noise, but it cannot have been doing that: depth_len accumulates lengths from the root, so d_i and d_j are always >= d_m, 2*d_m is exact, and rounding a non-negative exact result to nearest keeps it non-negative. Checked over every pair in 200 generated trees. What the clamp did catch was the real thing, a negative branch length, which neighbour-joining trees routinely carry. Those distances were reported as 0.0, claiming two distinct taxa sit on top of each other, in a matrix that then went on into clustering. Report the value as computed; the tree already gets a warning, now saying what to expect.
The branch filter was [main, "*.*.x", "*.x"], and a release branch named after its version matches neither glob, since '*.*.x' needs a literal '.x' suffix. Work on 1.0.1 has therefore never been checked by CI. Clippy also ran over the binary alone, leaving the test code unlinted, and the last step was a bare release build rather than a release test run, which only became worth doing once the integration tests stopped looking for the binary in target/debug.
The three example matrices had their header row absorbed into the opening code fence, so each block rendered with the label row missing and the rest of the table a column short. The numbers were invented too, and the topological one is not a tree: it puts LeafA and LeafD one edge apart, which no pair of leaves can be. Replace all three with the real output of one worked example, and add the --lower layout, which had no example at all.
Records what changed for anyone reading the README or the changelog rather than the diff: one tree per file, malformed input rejected instead of guessed at, the --precision ceiling, --midpoint ignored in topology mode, non-ASCII labels surviving, and negative branch lengths reaching the output.
Two things kept the matrix loop from using the cores it asked for. Each row was its own parallel job. A cell is one MRCA query and three array reads, so a row of a small matrix is nowhere near enough work to pay for synchronising a thread pool, and the fork/join per row dominated everything: an 8,000-tip run took 2.52s on 14 cores against 1.52s on one, with system time going from 0.10s to 14.32s. Rows are now computed in batches sized to about a million cells, so the synchronisation happens once per batch rather than once per row. The other two thirds of the time was formatting. Turning a float into a fixed number of decimals costs more than computing the distance it prints, and it sat in the serial write loop where no number of cores could reach it. Each worker now formats its own rows into a byte buffer and the writer only hands finished bytes to the operating system. 20,000 tips, -p 6 --lower: 10.61s -> 1.61s 8,000 tips, -p 6 --lower: 2.52s -> 0.20s and the run now scales with cores instead of against them (8,000 tips: 1.26s on one core, 0.21s on eight). Output is byte-identical. The batch buffer costs a fixed ~15 MB, which does not grow with the tree, and the output buffer goes from 8 KB to 1 MB so a multi-gigabyte matrix is not hundreds of thousands of write syscalls.
flatten_raw copies what it needs into Vec<Node>, so from that point the input text and the recursive parse tree are dead. They stayed in scope to the end of run() regardless, which on a large tree means carrying a second copy of it alongside the LCA table for the whole matrix loop, the longest part of the run. The measured effect is small (about 2 MB of resident memory on a 100,000-tip tree) because the allocator holds on to the freed pages rather than returning them, but the pages are reclaimable once released and were not before.
Fourteen pages of MkDocs Material, published to GitHub Pages from main.
The README had grown into the whole manual and was the only place any of
this was written down, which made it long enough that nobody reads to the
end and still left no room for the things that need a page of their own.
Getting started installation, and the commands most runs use
User guide input, the three distance modes, midpoint rooting,
output formats, and a full CLI reference
How it works the parser, the LCA structure, the streaming loop,
and what the run costs in time and memory
Recipes PCoA, transmission clusters, PGLS, neighbour-joining
About changelog, citation, contributing
Every matrix on the site is real output rather than a plausible-looking
table, every error message is quoted from a run, and the benchmark
figures are measured on a stated machine with the estimates marked as
estimates.
The site builds with --strict, so a dead link or a missing nav entry
fails CI rather than shipping. Pull requests build without deploying.
A table of the nine pages people actually go looking for, right under the tagline. The rest of the README stays as it is, so anyone reading it on GitHub without following a link still has everything.
A leaf label with a space passed every check and then broke --lower. A PHYLIP row is a name followed by whitespace and then the values, so a reader splitting on whitespace reads 'Taxon A<TAB>4.5' as three fields and lands every row after it one column out. Tabs and newlines were already refused for the same reason; whitespace is refused now too, but only in --lower mode, since the square TSV is tab-delimited and reads it back correctly. --lower with --lmm silently dropped data. Omitting the diagonal costs nothing on a distance matrix, where it is all zeros, but an LMM diagonal holds each leaf's root-to-tip length, which is what the off-diagonal covariances have to be read against. It now says so. A gzipped tree failed with 'stream did not contain valid UTF-8', which says nothing about what to do. Large trees usually arrive compressed, so check for the magic bytes and give the gunzip line instead.
The guide pages explain what each flag does, and the recipes give snippets to lift, but neither walks anyone through a whole question from tree to answer. Seven M. tuberculosis isolates, ten steps, and the question the tool exists for: which of these are transmission links. It goes from the Newick through the matrix, the substitutions-to-SNPs conversion, single-linkage clustering at the two standard thresholds, nearest neighbours, the topological and variance-covariance matrices, and what changes when the dataset gets big. The two thresholds are the point of it. At 5 SNPs TB_003 is outside the cluster and at 12 it is inside, which is a decision to report rather than an output to read off, and no amount of precision settles it. Every matrix on the page is real output from the tree the page ships, and each command was run as written.
Every check in the suite so far was written against the same understanding of the problem as the code: midpoint rooting preserves distances, the binary-lifting MRCA agrees with walking up from both nodes, a patristic distance is never negative. All true, all necessary, and none of it would catch the definitions themselves being wrong. scripts/crossvalidate.R compares distree against ape over generated trees of mixed shape (random, ultrametric, with polytomies), in all four modes: cophenetic.phylo for patristic, vcv.phylo for the variance-covariance matrix, cophenetic over unit branch lengths for edge counts, and phangorn::midpoint for the rooting. Over 250 trees the worst disagreement is 9.6e-10 for the float modes, which is the 12-decimal text round-trip, and exactly zero for edge counts. That the midpoint agrees with phangorn is the reassuring part. It is the only mode where rooting changes the answer and the only code in the tool that rewrites the tree. R is a heavy thing to need for running 59 tests test lca::tests::test_depth_top ... ok test lca::tests::test_mrca_deeper ... ok test midpoint::tests::test_midpoint_asymmetric ... ok test lca::tests::test_mrca_self ... ok test midpoint::tests::test_midpoint_simple ... ok test parser::tests::test_apostrophe_in_comment ... ok test midpoint::tests::test_midpoint_preserves_distances ... ok test lca::tests::test_mrca_siblings ... ok test parser::tests::test_apostrophe_in_unquoted_label ... ok test parser::tests::test_bracket_in_label_position ... ok test parser::tests::test_comment_before_label ... ok test parser::tests::test_comment_before_subtree ... ok test parser::tests::test_empty_branch_length_error ... ok test parser::tests::test_empty_input_error ... ok test parser::tests::test_escaped_double_quotes_in_labels ... ok test parser::tests::test_escaped_quote_keeps_inner_whitespace ... ok test parser::tests::test_escaped_single_quotes_in_labels ... ok test parser::tests::test_free_text_is_not_a_leaf ... ok test parser::tests::test_flatten ... ok test parser::tests::test_multiple_trees_error ... ok test parser::tests::test_leading_rooting_comment ... ok test parser::tests::test_negative_branch_length_parsed ... ok test parser::tests::test_nested_brackets ... ok test parser::tests::test_newlines_in_newick ... ok test parser::tests::test_no_trailing_semicolon ... ok test parser::tests::test_non_ascii_labels_preserved ... ok test parser::tests::test_parse_internal_labels ... ok test parser::tests::test_parse_nhx_comments ... ok test parser::tests::test_parse_no_branch_lengths ... ok test parser::tests::test_parse_quoted_labels_double ... ok test parser::tests::test_parse_quoted_labels_single ... ok test parser::tests::test_parse_scientific_notation ... ok test parser::tests::test_parse_simple_tree ... ok test parser::tests::test_quoted_internal_node_label ... ok test parser::tests::test_trailing_comment_allowed ... ok test parser::tests::test_trailing_content_error ... ok test parser::tests::test_unclosed_comment_error ... ok test parser::tests::test_unclosed_quote_error ... ok test parser::tests::test_unmatched_paren_error ... ok test parser::tests::test_whitespace_in_newick ... ok test tests::test_duplicate_leaves_detected ... ok test tests::test_large_symmetric_tree ... ok test tests::test_lmm_matrix ... ok test tests::test_mode_conflict ... ok test tests::test_negative_branch_length_detected ... ok test tests::test_negative_branch_length_gives_negative_distance ... ok test tests::test_lower_triangle_row_counts ... ok test tests::test_patristic_clamped_nonnegative ... ok test tests::test_no_branch_lengths_all_zero ... ok test tests::test_single_leaf ... ok test tests::test_topology_distances ... ok test tests::test_trifurcation ... ok test tests::test_simple_patristic_distances ... ok test tests::test_zero_distance_same_leaf ... ok test parser::tests::test_deep_tree_no_stack_overflow ... ok test tests::test_patristic_never_negative_on_random_trees ... ok test midpoint::tests::test_midpoint_random_trees ... ok test parser::tests::test_deeply_nested_tree_drops_without_overflow ... ok test lca::tests::test_mrca_matches_walking_up_on_random_trees ... ok test result: ok. 59 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.56s running 4 tests test test_topology_matches_ape_unit_branch_cophenetic ... ok test test_patristic_matches_ape_cophenetic ... ok test test_lmm_matches_ape_vcv ... ok test test_midpoint_leaves_patristic_alone ... ok test result: ok. 4 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.26s running 17 tests test test_binary_stdin ... ok test test_binary_empty_input_error ... ok test test_binary_rejects_out_of_range_precision ... ok test test_binary_duplicate_leaf_error ... ok test test_binary_rejects_zero_threads ... ok test test_binary_rejects_newline_in_label ... ok test test_binary_names_gzip_input ... ok test test_binary_keeps_output_file_on_parse_error ... ok test test_binary_basic_patristic ... ok test test_binary_lower_triangle ... ok test test_binary_precision_flag ... ok test test_binary_topology_flag ... ok test test_binary_midpoint_does_not_inflate_topology ... ok test test_binary_midpoint_preserves_patristic ... ok test test_binary_rejects_whitespace_label_in_lower_mode ... ok test test_binary_warns_about_unlabeled_leaves ... ok test test_binary_warns_lower_drops_lmm_diagonal ... ok test result: ok. 17 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s, so six of ape's matrices are committed under tests/fixtures/ and tests/crossvalidation.rs checks those everywhere. The full run is a weekly job and a button.
src/main.rs held the whole tool: argument handling, the Newick parser, the LCA structure, the distance definitions and the output loop. Nothing outside the binary could reach any of it, which rules out fuzzing the parser, generating API documentation, and using distree from another Rust program. src/lib.rs is now the tree side (parser, tree, lca, midpoint, DistMode and compute_distance) and src/main.rs is the command line over the top: flags, input, warnings, formatting, output. The unit tests for the distance definitions moved with the code they test. No behaviour changes; the output is byte-identical.
The torture test found this on its first run. 'A:1e910' parses: Rust returns infinity for an exponent past f64's range rather than an error. An infinite depth then makes every distance infinite and the diagonal NaN, since inf - inf is NaN, and the whole thing came out with exit code 0 as a matrix of inf and NaN. Two guards, because there are two ways in. The parser now refuses a branch length that is not finite, which is the case anyone will actually hit. And since a distance is d_i + d_j - 2*d_m, a depth past half of f64's range overflows that sum even when every individual length is finite, so the run also stops if the deepest leaf is far enough out for that: '((A:1e308,B:1e-308):1e300,C:0.0);' used to produce a NaN diagonal and now says to rescale the tree. Trees with large but usable lengths are untouched: 1e100 still works.
Two ways in, because they catch different things. tests/torture.rs runs on every push and needs no extra toolchain. It takes valid trees and truncates, flips, duplicates and swaps bytes in them, the way a real Newick file gets damaged, then asserts only that nothing panics and nothing hangs. It found the infinite-branch-length bug on its first run. fuzz/ is the coverage-guided version, for when there is time to leave it going. Two targets: the parser alone, and everything that parses pushed through flattening, rooting and all three distance modes. 12.4M runs on the first and 5.1M on the second, no crashes, starting from the seeds in fuzz/seeds/.
Three things people kept having to work around. Large trees arrive compressed, and distree made you pipe them through gunzip. It now decompresses them itself, detected by magic bytes rather than by extension so it works from stdin too. --taxa FILE restricts the matrix to the labels listed in a file, one per line, blanks and # comments skipped. It filters the output rather than pruning the tree, which is the point: the path between two leaves does not depend on which other leaves are in the matrix, so a subset carries the same distances as the full run. A label that is not in the tree is an error naming it, since a quietly smaller matrix is the kind of thing nobody notices until much later. --stats prints a summary to stderr: leaves, nodes, mode, cells, and the minimum, maximum and mean off the diagonal. The workers accumulate it as they go, so it costs one pass and nothing measurable.
Text is an expensive way to move a large matrix. At twelve decimals a cell is fourteen bytes and most of the run is spent producing them; as a 64-bit float it is eight bytes, exact, and costs nothing to write. On an 8,000-leaf tree, to a real file: --lower -p 12 0.26s 458 MB --lower --npy 0.10s 244 MB -p 12 0.56s 916 MB --npy 0.19s 488 MB so 2.6x faster and 47% smaller, at full precision rather than twelve decimals. numpy.load reads it directly. --lower switches it to the condensed vector rather than PHYLIP's lower triangle, because those are not the same triangle: SciPy reads the upper one row by row, and emitting PHYLIP's order would hand squareform the right values in the wrong places. Checked against squareform in the tests. .npy has nowhere to put labels, so they go to <FILE>.labels.txt in row order, which is why --npy needs -o.
write!("{:.p$}") expands the float to its exact decimal form and goes
through core::fmt to print it. That is correct and it was about half the
cost of a text run. Scaling by a power of ten and emitting the digits
directly is roughly seven times faster at the formatting itself:
--lower -p 6 0.22s -> 0.13s
--lower -p 10 0.24s -> 0.15s
-p 10 (square) 0.47s -> 0.29s
The reason a shortcut like this is normally a bad idea is that
multiplying by 10^p rounds once, and where that carries the product
across the nearest .5 boundary there is no way to tell from here which
way the exact value rounds. So it does not guess: it measures its
distance from the boundary against the error bound, and anything inside
that goes to the standard formatter. Roughly one value in a thousand of
real distance data takes that path, and every exact tie does.
Verified rather than assumed. The unit tests check the fast path against
the standard formatter over 200,000 random values across 24 orders of
magnitude, straight down the middle of the rounding boundaries at every
precision, over distance-shaped values, and on subnormals, 2^53, signed
zero and the rest. Separately, 612 matrices across 11 trees, 17
precisions and both float modes are byte-identical to the old output.
The first version of this got signed zero wrong: a small negative value
that rounds to zero prints as -0.00, and taking the sign from the rounded
integer loses it. The boundary test caught it.
Covers --npy, --taxa, --stats and gzipped input across the CLI reference, input, output and recipes pages, and refreshes the benchmark tables now that the formatter is faster. Also records what the fuzzing and the ape cross-validation are for, since they are the part of the suite that would otherwise look like more of the same.
It was 329 lines and had become a second copy of the documentation site: the full flag reference, the output formats, five worked use cases, the troubleshooting list. Two copies of the same thing only stay in step while someone is watching, and this one had already drifted. Its Usage block listed neither --npy nor --taxa nor --stats, and said nothing about reading gzip, so anyone reading the README on GitHub got a flag reference that the site contradicted. What is left is what a landing page is for: what the tool does, where the documentation is, how to install it, eight commands worth copying, a performance table and how to cite it. The detail lives on the site, which is the only copy now. Every command in the quick start was run as written.
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
1 participant
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.
Everything for the 1.0.1 release.
mainis still at 1.0.0, so this is the wholeversion: the parser rewrite and bug fixes from earlier work on the branch, plus
a correctness audit, the performance work, three new flags and a documentation
site.
Bugs fixed
The expensive class here is not a crash, it is a run that returns a matrix that
looks entirely reasonable and is wrong. Most of what follows is that.
Parsing
[&R]/[&U]rooting marker, which IQ-TREE, MrBayes and BEAST allwrite, made the tree unparseable: comments were only skipped after a label,
so
[&R] (A,B);died with "No labeled leaves found in the tree". Per-branchmetadata written before the label failed the same way.
byte as char,Señor_Ñucame backout of the matrix as mojibake, so any label with an accent, a Greek letter or
CJK text silently stopped matching the sample names used downstream.
((A:1,B:2),C:3with the last)missing hadits open parentheses closed at end of input, and every internal node left
dangling quietly lost its branch length.
saying so. So did trailing junk after the tree, and an unclosed
[swallowedthe rest of the file as a comment.
(was not a label delimiter, so free-form prose parsed as a single leafnamed after it.
O'Brienbrokeparsing.
A:1e910parsed as infinity, because Rust returns infinity for an exponentpast f64's range rather than an error. Every distance came out
inf, thediagonal
NaN, and the exit code said success.made iterative in earlier work on this branch, but
RawNode's drop glue wasnot, so a 300,000-level tree aborted after parsing had succeeded.
Output
-o FILEwas created and truncated before the tree was read, so a run thatthen failed on a malformed tree left an empty file where the previous matrix
had been.
and exit code 0. A closed pipe went the other way, printing an error for
distree tree.nwk | head.does; only tabs were rejected.
--lower: PHYLIP readers treat whitespaceas the end of the name, so every row after it landed a column out.
--lower --lmmdropped the diagonal, which under--lmmis each leaf'sroot-to-tip length rather than zeros.
the tree.
Arguments and modes
--midpoint --topologyinflated every crossing distance by one hop: the nodeinserted at the midpoint splits an edge in two. Rooting cannot change an edge
count, so it is skipped there now.
taxa were the same sample. Neighbour-joining trees carry negative branches
routinely. The clamp could not have been doing what it was for: rounding
cannot make that subtraction negative.
-p 50000000panicked inside the formatter;-t 0reached Rayon, which reads0 as "use the default" and started a thread per core.
Infrastructure
[main, "*.*.x", "*.x"], and1.0.1matches neither glob.target/debug/distree, socargo test --releaseexercised a stale debug binary and reported a pass forcode it never ran.
code fence, and the topological one was not a realisable tree: it put two
leaves one edge apart.
New
--npywrites the matrix as a NumPy array of 64-bit floats: exact, halfthe size of equivalent text, and 2.6x faster. With
--lowerit writes thecondensed vector SciPy reads directly. Labels go to
<FILE>.labels.txt.--taxa FILErestricts the matrix to a list of leaf labels. It filtersthe output rather than pruning the tree, so the distances are the ones the
full tree gives, which is the opposite of what pruning first would produce.
--statsprints a summary to stderr: leaves, nodes, mode, cells, and theminimum, maximum and mean off the diagonal.
name, so it works from stdin too.
Performance
Two things kept the parallelism from paying. Each row was its own parallel job,
and a row of a small matrix is nowhere near enough work to pay for synchronising
a thread pool: an 8,000-leaf run took 2.52s on 14 cores against 1.52s on one.
The other two thirds of the time was float formatting, sitting in the serial
write loop where no number of cores could reach it.
Rows are now computed in batches, each worker formats its own, and fixed-
precision formatting is done by hand where the rounding is unambiguous and
handed to the standard formatter where it is not.
-p 6 --lowerA 20,000-tip matrix went from 10.6s to 1.0s. The LCA table also stores plain
usizerather thanOption<usize>, halving the largest allocation the toolmakes: about 350 MB on a million-leaf tree.
Output is byte-identical throughout. Verified across 612 matrices spanning 11
trees, 17 precisions and both float modes.
Correctness
The suite was self-consistent: it asserted that midpoint rooting preserves
distances, that the binary-lifting MRCA agrees with walking up from both nodes,
that a patristic distance is never negative. All necessary, all written against
the same understanding of the problem as the code, so none of it would catch a
distance being defined wrongly.
scripts/crossvalidate.Rcompares against ape over generated trees:cophenetic.phylofor patristic,vcv.phylofor variance-covariance,cophenetic.phyloover unit branch lengths for edge counts, andphangorn::midpointfor the rooting. Over 250 trees the worst disagreement is9.6e-10, which is the 12-decimal text round-trip, and exactly zero for edge
counts. That the midpoint agrees with phangorn is the reassuring part: it is the
only code in the tool that rewrites the tree.
Six of ape's matrices are committed under
tests/fixtures/, socargo testchecks the same thing without needing R. The full run is a weekly job.
The parser and the pipeline are also fuzzed, in the test suite on every push and
with
cargo fuzzfor longer runs. That is what found the1e910bug.96 tests, up from 31.
Documentation
A MkDocs Material site under
docs/, published to GitHub Pages: gettingstarted, a tutorial working through an outbreak investigation from tree to
transmission clusters, a user guide, how it works, recipes. Every matrix on the
site is real output and every error message is quoted from a run.
The README is now a landing page pointing at it. It had grown into a second copy
of the manual and had already drifted: its flag reference listed none of the new
flags.
Behaviour changes worth knowing about
The parser is stricter, and these were all previously accepted:
--lowermode.--threads 0and--precisionabove 30 are rejected.On merge
.github/workflows/docs.ymldeploys on a push tomain, so merging publishesthe site at https://pathogenomics-lab.github.io/distree/ once Pages is enabled
(Settings → Pages → Deploy from a branch →
gh-pages). Taggingv1.0.1buildsthe release binaries.