Repository navigation
Perf batch - #13
Open
jeromekelleher wants to merge 6 commits into
Open
Perf batch#13jeromekelleher wants to merge 6 commits into
jeromekelleher wants to merge 6 commits into
Conversation
Profile (perf record, line-level annotation of tsls_hmm_find_path_matrix,
into which the site update, coalescing and walks are inlined) of chr20p
sample matching, as shares of that function's samples:
map (2 queries) constant r (3)
allele walk (205-211) + clear (225) ~30% ~27%
coalesce walk to inherited value (1343) ~16% ~12%
coalesce cache writes (1354) ~8% ~10%
cache reset walks (1021, 1412) ~14% ~15%
traceback run (2269) ~2% ~5%
arithmetic (1615) <1% <1%
tsls_hmm_update_tree is a further ~10% of the run under the map and ~25%
under constant r. The time is in walks up the tree, not in the values.
(An all-missing query is not a useful floor for the topology cost: it
takes 15 s, because nodes entering the tree at 0 never coalesce.)
The caches are now valid on nodes whose cache_stamp (in the node record,
which grows from 8 to 16 bytes so records don't straddle cache lines)
equals the HMM's, and clearing them all is an increment, so the reset
walks after edge removals, coalescing and sparse sites are gone. A node
with a likelihood ignores any stale cache.
Paired wall time (ab.py: alternating processes pinned to one core, best
of 3 per query, 6 samples), stamp / base: map 0.908, 0.935, 0.939;
constant r 0.957, 0.975, 0.980. Paths identical.
benchmarking/profile_samples.py runs and times tsls on chr20p samples
under a map or constant r; benchmarking/ab.py compares builds with it.
Every node with a mutation at a site is in the likelihood list (tsls_hmm_insert_mutation_likelihoods), so a listed node without a mutation has the allele of its nearest listed ancestor. At the start of each Viterbi site, one memoised walk finds each listed node's nearest listed proper ancestor (cached on the unlisted nodes passed, with the likelihood cache's stamp, in the node record's spare field). Alleles are then resolved by walking up the list's own tree, and coalescing compares each node with its listed ancestor's value and flag, so the allele walks up to a mutation or root, and the separate coalescing walks, are gone. Removals are applied after all the comparisons, since a removed node had the same value and flag as its own ancestor. Forward and backward are unchanged. Paired wall time (ab.py, 6 chr20p samples, best of 3), over the build before the stamps: map: stamp 0.925-0.936, listed 0.719-0.726 constant r: stamp 0.991-0.997, listed 0.849-0.916 Paths identical.
The walk to each listed node's nearest listed ancestor was ~50% of
tsls_hmm_find_path_matrix after the previous change (perf, chr20p samples
under the map), and each step read two records: the likelihood, to see
whether the node is listed, and the node record, for the parent and
cache. Each listed node is now first marked with the cache stamp and
itself as its listed_ancestor, so the walk stops at a marked node and
reads only the node records. The proper ancestors are collected in a
scratch array and written back after all the walks.
Paired wall time (ab.py, 6 chr20p samples, best of 3):
map: 0.793, 0.858, 0.864 of the previous build
constant r: 0.748-0.765 of the build before the stamps
(previous build 0.809-0.828)
Paths identical.
The left_child, right_child, left_sib and right_sib arrays were only used by the in-tree test and by tsls_hmm_number_tree (sparse sites), but every edge insertion and removal updated them, touching several cache lines. The tree is now the parent links plus a count of children per node. tsls_hmm_number_tree lists the children with a counting sort over the parent array when it numbers a tree, which is O(N) like the numbering itself; children are now visited in node ID order rather than insertion order, which can only change sparse sites' choice between equally likely nodes. Measurements before this change (perf, chr20p samples under the map): the walk to listed ancestors was ~40% of tsls_hmm_find_path_matrix (~31% of the run), and tsls_hmm_update_tree plus tree position stepping ~7%. Walks averaged 271-280 steps per site under the map (109-123 listed nodes, 56-58% of them with a listed parent) and 120-129 under constant r (14-28 listed); 7.2 edges out and 7.2 in per site. Paired wall time (ab.py, 6 chr20p samples, best of 3), over the previous build: map 0.994, 0.956, 0.957; constant r 0.943, 0.936, 0.929. Paths identical. (A guard-rail run on sc2ts was invalidated by rebuilding the extension mid-run; sc2ts is checked after the next change.)
Every walk up the tree now goes to the nearest node with a likelihood (tsls_hmm_find_listed) and caches the answer, as a node ID, on the nodes without likelihoods that it passes. Nodes with likelihoods are always marked (the cache stamp, and themselves as listed_ancestor), so walks read only node records. The caches are no longer cleared after each phase: a cached answer stays right until a node on a cached path gets a likelihood or a new parent. So: - a node given a likelihood that has a cached walk clears all the caches (and the list is marked again), since walks from below may have passed through it; - an edge removal gives the child a likelihood first, which does the same; - a node losing its likelihood (coalescing) becomes a cached node pointing to its old nearest listed ancestor, and walks follow such pointers until a node with a likelihood; - nodes leaving the tree are isolated, so no walk ends there. The value caches (L.cache, recombination_required_cache) are gone: the inherited value and flag are read from the listed ancestor. In Viterbi, each listed node's proper listed ancestor is kept in its likelihood record, for the alleles and coalescing. tsls_hmm_check_state checks that every cached walk leads to the nearest listed ancestor. Measured before implementing, by checking the previous site's stamps: a full clear would be needed at 24% of sites under the map (mutation node on a cached path 9%, an edge-removal child on one 19%) and 14-15% under constant r. Paired wall time (ab.py, 6 chr20p samples, best of 3), over the previous build: map 0.732, 0.719, 0.716; constant r 0.668, 0.698, 0.697. Paths identical.
Add a benchmarking README section on this round of algorithmic work on sample matching under a recombination map: the profile it started from, each change and its paired timing, the end-to-end result (0.45-0.47 of the earlier build under the map, 0.50-0.52 under constant r, identical paths), the guard-rail workloads (ancestor groups 84 and 1485, sc2ts 100k; all faster, path classes unchanged), and the dead ends and remaining ideas, each estimated under 10%. Dead end recorded here (not committed): reusing each listed node's own ancestor across sites, stamped and recomputed when stale, gave 1.00-1.02 of the previous build under the map, since checking the stored ancestor costs a record load, as the one-step walk it replaces does.
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
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.
No description provided.