Repository navigation
Remove partial evaluation; fix conditional target_vars for DiscreteLogistic - #59
Merged
Merged
Conversation
…tion `conditional(pc, ..., target_vars = T)` restricted its input-layer kernels to the target nodes through `layer.enable_partial_evaluation` and a `partial_eval` constexpr. The DiscreteLogistic kernel then read every target node's parameters and metadata through its POSITION in the target list instead of its node id, so each node was combined with another node's mean, scale and value range: up to 0.34 off in probability, with every row still summing to 1. (The Categorical path's version of this, issue #21, was fixed by #40.) The query now builds its own explicit node list (`_target_nodes`: the nodes whose variable is a target, via torch.isin over layer.vids) and both kernels always index through it -- all nodes for the full query -- so parameters, metadata and flows are read at the node's own id by construction. Restricting to the target nodes is kept, so target_vars stays cheap for large vocabularies. target_vars_test.py checks target_vars against the full conditional for Categorical and DiscreteLogistic leaves on HMM, HCLT, PD and RAT-SPN, in both call orders, and that a target_vars query leaves later passes unchanged; the DiscreteLogistic cases fail before this change. Closes #21.
Partial evaluation restricted forward/backward passes to the nodes whose scope lay inside a given set of variables. It has not been used for a long time, has been the source of silent bugs (a CUDA graph replayed across a toggle was off by 38 nats; input layers used block ids as node offsets, #40), and its last internal user -- conditional's target_vars -- no longer needs it. BREAKING: removes TensorCircuit.enable_partial_evaluation / disable_partial_evaluation and the same methods on LayerGroup, Layer, ProdLayer and InputLayer, together with the scope-to-node-id caches that only fed them (_create_scope2nid_cache, every layer's _prepare_scope2nids, fw/bk_scope2localids, scope2localgids) and the _fw/_bk_partial_eval_enabled flags. The layers' node-subset code paths are now dead (their selections are always None) and go in the next commit, together with the kernels' partial_eval branches. Removed the partial-evaluation tests and the CUDA-graph test for the enable/disable toggle; the target_vars case of input_layer_partial_eval_block_test is covered by tests/queries/conditional/target_vars_test.py (same 1-block-per-group input layers). Full suite: 1252 passed, 26 skipped.
…and their kernels With the partial-evaluation API gone, the layers' node-subset paths were dead: their selections (fw/bk_partition_local_ids, bk_fw_partition_local_ids) were always None. This removes them and what they fed: * 18 Triton kernels in layer/kernels/ lose their `local_ids` pointer and `partial_eval` constexpr together with the remap block it guarded. The constexpr was always 0, so the compiled arithmetic is unchanged. * SumLayer / ProdLayer forward and backward keep only the whole-layer loop; the internal methods lose their `local_ids` parameter, the launch sites their `local_ids` / `partial_eval` arguments, the CUDA fast-path gates their `local_ids is None` conditions, and the autotune cache keys their always-zero `partial_eval` entry. * BREAKING: ProdLayer.forward no longer takes `_for_backward`. It only chose the partial-evaluation node subset for the forward recompute inside the backward pass; the recompute is now the ordinary forward. Updated its callers (TensorCircuit.backward, the partition-gradient pass and the tests that recompute a layer by hand). * ExternalParamsSumLayer drops its "not supported under partial evaluation" guard. Verified against main with the eager differential harness: with autotuning off every deterministic output is bit-identical (the rest is atomic-add noise within main's own spread); with autotuning on, isolated reruns match main. Full suite: 1252 passed, 26 skipped.
…butions The last of the partial-evaluation plumbing, all dead since the API was removed (the input layers' fw/bk_local_ids were always None): * InputLayer: the forward/backward node selections, the class-level fw/bk_local_ids defaults, the `bk_local_ids is None` term of the CUDA backward gate, and the partial-evaluation mask in _backward_missing. The forward, missing-mask and flow kernel templates lose their local-ids pointer, `partial_eval` constexpr and the remap they guarded. * BREAKING for custom distributions: the launch arguments handed to post-processing kernels (`_fw_launch_common` / the backward's `_bk_launch_common`) no longer contain `fw_local_ids_ptr` / `bk_local_ids_ptr` and `partial_eval`; a custom post kernel must drop those parameters. Every built-in distribution is updated: ExternProductCategorical (8 kernels, whose `else` branch -- the non-partial offset -- is kept as the plain assignment, and 6 host-side selections), SoftEvidenceCategorical (6 kernels, 3 guards, 3 asserts), LatentSoftEvidence, External and SoftEvidenceIndicator. * queries.conditional: the soft-evidence forward kernels' always-False `partial` flag and its node-list branch. * docs: the API reference no longer lists the removed enable/disable_partial_evaluation (missed in the API commit). Verified against main with the eager differential harness (every distribution above is exercised): bit-identical or within main's own atomic-add spread. Full suite: 1252 passed, 26 skipped.
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.
Summary
This PR removes partial evaluation: the feature that restricted forward and backward passes to the nodes whose scope lies inside a given set of variables. It had not been used for a long time, and it had produced silent bugs:
conditional(target_vars=...)was wrong forDiscreteLogisticleaves, as described below.Its last internal user was
queries.conditional'starget_vars, which now builds its own node list. The removal makes PCs 0–4% faster in eager mode and changes no results.Closes #21.
Bug fix:
conditional(..., target_vars=...)with DiscreteLogistic leavesconditional(pc, ..., target_vars = T)restricted its input-layer kernels to the target nodes through partial evaluation. The DiscreteLogistic kernel then read each target node's parameters and metadata through the node's position in the target list instead of its node id. So every node was combined with another node's mean, scale and value range. The error was up to 0.34 in probability, while every row still summed to 1. The Categorical path's version of this bug was #21, fixed by #40; the DiscreteLogistic path still had it.The query now builds an explicit list of the nodes whose variable is a target. Both conditional kernels always index through it (for the full query it is simply all nodes), so the bug can't recur, and
target_varsstays cheap for large vocabularies.The new test
tests/queries/conditional/target_vars_test.pycheckstarget_varsagainst the full conditional:The DiscreteLogistic cases fail on
mainand pass here.Breaking changes
TensorCircuit.enable_partial_evaluation/disable_partial_evaluationare removed, together with the same methods onLayerGroup,Layer,ProdLayerandInputLayer, and the scope-to-node-id caches that only fed them.ProdLayer.forwardno longer takes_for_backward. It only selected the partial-evaluation nodes for the forward recompute inside the backward pass; that recompute is now the ordinary forward. A caller still passing it lands in**kwargsand gets the identical result.fw_local_ids_ptr/bk_local_ids_ptrorpartial_eval, so such kernels must drop those parameters. Every built-in distribution is updated.What changed internally
layer/kernels/(sum forward and backward, product) and the input-layer kernel templates lose their local-id pointer, theirpartial_evalcompile-time constant, and the remap it guarded. The constant was always 0, so the compiled arithmetic is unchanged.local_ids is Noneconditions, and the autotune cache keys lose an entry that was always zero.Falsepartialflag inqueries.conditional's soft-evidence forward kernels.target_varscase ofinput_layer_partial_eval_block_test.pyis covered by the new test on the same kind of input layers (one block per input group). The API reference no longer lists the removed methods.Verification
Output equivalence: an eager differential harness compares 27 workloads (110 outputs) against
main. These cover forward, backward, EM, MPE, tempering, sparse mode, HCLT, PD, RAT-SPN, soft evidence, BlockScale, LowRank, sampling,marginal/conditional, and every distribution touched here.main's own run-to-run spread, or timing-based kernel choices that matchmainwhen rerun in isolation.Full test suite after each commit: 1252 passed, 26 skipped.
Speed, eager forward+backward step,
main→ this PR, interleaved runs on the same GPU:Less per-launch host work; on the last three workloads every run of this PR was faster than every run of
main.Training:
mainand this PR, with validation log-likelihood −0.8074 vs −0.8077. Measured on its own, the PC's forward+backward takes 5.42 ms onmainand 5.30 ms on this PR.Commits