Add PCA dimensionality reduction to the anvil pipeline - #592
Merged
Conversation
PCA dimension reduction was missing from the anvil pipeline while feature matrices keep growing (2048-bit fingerprints, descriptor blocks), so models train slowly on redundant dimensions. PCATransform fits one PCA over the whole matrix (int n_components) or one PCA per feature block (dict of block key to component count, keys validated two-way against the featurizer block layout) and can impute NaNs ahead of the PCA since PCA itself cannot see NaN. Fit validates block keys, duplicate keys, exact width coverage, and component counts against the train data; transform validates the fitted column layout so wider or narrower matrices fail loudly. TransformBase gains an accepts_feature_blocks flag so workflows can forward block layouts only to transforms that use them, and the module-level helpers fit_transforms / transform_features fit or apply a single transform or an ordered sequence by threading each element's output into the next. ImputeTransform.fit now returns self as its docstring always claimed, so chained usage works.
Per-block transforms (e.g. per-featurizer PCA with different
dimensionalities) need to know where each source featurizer's columns
sit inside a concatenated matrix, but featurizers only exposed the
merged array.
FeaturizerBase.feature_blocks reports the column layout as (key,
width) pairs in matrix order, defaulting to a single block keyed by
the registry name whose width is probed from the first featurizable
row; the probe skips rows that fail featurization and raises only
when nothing probes. FeatureConcatenator flattens child layouts
recursively in concatenation order so nested concatenators stay
aligned with the emitted columns.
While there, FeatureConcatenator accepts the {type, params} list
entry form already used by anvil section YAMLs (single-key entries
and instances keep working) and rejects same-class featurizers, which
would produce same-key blocks that per-key transforms cannot
disambiguate.
Transforms were fit and re-applied by hand and were lost in decoupled inference: predict only featurized, so any model trained with a transform consumed the wrong features after training. The sklearn workflow now probes the featurizer block layout on the train rows, fits the transform sequence on the train partition only (no fit leakage into val/test), applies it to val/test/full, and persists the fitted transforms next to the model as a schema-versioned pickle; the recipe YAML carries configuration only. Transform sections accept a single transform or an ordered list, with the global seed filling entries that set none. Inference loads the saved artifact when the recipe declares a transform (missing artifact or unknown schema is a hard error, never a silent skip), returns it from load_anvil_model_and_metadata (4-tuple becomes a 5-tuple), and predict applies it before the model with a row-count boundary check and single-row 1D normalization. The sklearn workflow rejects DataLoader-emitting featurizers with a clear message, and the posthoc comparison label builder handles the list feature section form.
The transform path had no end-to-end coverage on a real dataset. Two CPU recipes train LGBM on the 1000-row CYP multitask set: a single 256-component PCA over ECFP-4 fingerprints, and a stacked ImputeTransform plus per-block PCA (256 fingerprint components, 32 descriptor components) over a 2D-descriptor-plus-fingerprint concatenation, which also exercises the list feature form and the NaN-imputation step.
Codecov Report❌ Patch coverage is 🚀 New features to boost your workflow:
|
for more information, see https://pre-commit.ci
A rule pass over the 44 new tests found four groups expressing one contract across several bodies: n_components construction validation, the to_workflow seeding behavior, and the transform section parse shapes, plus a list-entry-form test whose mixed list proved nothing beyond the per-form parses. Each is now a single parametrized test (44 functions reduce to 41) that reuses the existing spec builder helper. The same pass showed the boolean n_components guard is unreachable: pydantic coerces True to 1 during int-or-dict type validation, before the field validator runs, so the guard and its test case are removed rather than testing a path that cannot fail. fit and transform docstrings gain *args/**kwargs descriptions for the D417 gate.
The ruff 0.16.2 pinned by pre-commit formats this pre-existing cell differently (single to double quotes), which blocked pre-commit run --all-files for the whole branch.
Both the single-transform and transform-sequence cases now flow through the same (section_spec, component) seeded_sections loop instead of the sequence case separately checking a None sentinel on the built component. TransformSpec already carries its own params dict, so _section_sets_seed works per-item once transform_spec/transform are normalized to lists.
…ntion TransformSpec.transform and AnvilWorkflowBase.transform were typed as T | list[T] | None, forcing single-vs-list branching everywhere they were consumed (to_workflow, seed-fill, saved recipe YAML). Every other optional section in these models (ensemble, param_paths, serial_paths) follows the same shape instead: None for absence, otherwise always the collection type. Both fields are now list[T] | None, with a mode="before" validator that wraps a bare mapping or transform instance into a one-element list, so existing single-transform YAML and direct Python construction keep working unchanged. to_workflow's transform handling and the seed-fill block from the previous commit both simplify since transform is always a list or None, never a bare instance.
target_cols already had a mode="before" validator wrapping a bare string into a one-element list, so self.target_cols was always a list[str] by construction time. The Union[str, list[str]] annotation and the two isinstance branches in _read_multiple_resources/_read_single_resource were defending against a shape that could no longer occur post-validation.
for more information, see https://pre-commit.ci
The one-line summary didn't explain why the sklearn workflow needs a numpy-producing featurizer or what failure mode this catches (a deep learning featurizer like ChemProp's paired with an sklearn-family model like XGBoost previously failed late inside the trainer instead of at spec-construction time).
Clarify why label_and_task_name_from_anvil handles both the dict and ordered-list forms of FeatureConcatenator.featurizers when parsing a saved anvil_recipe.yaml.
_parse_featurizer_entry hand-rolled shape detection and error raising for the two accepted list-entry forms. FeaturizerEntry (a BaseModel with extra="forbid" and a before-validator coercing the single-key form) matches the same normalize-via-before-validator pattern already used for TransformSpec and DataSpec.target_cols, and gets extra-key rejection for free instead of a manual set-difference check.
feature_blocks previously rediscovered each featurizer's column width by featurizing single probe inputs one at a time, skipping ones that failed. Only FeatureConcatenator ever consumes block boundaries (per-block PCA needs them; a bare leaf featurizer never does, since int n_components never checks feature_blocks), so the probing lived on FeaturizerBase for no real reason and duplicated featurization work FeatureConcatenator was already doing in its own featurize() loop. feature_blocks is now a thin accessor on FeatureConcatenator that returns the block list recorded as a side effect of the most recent featurize() call, and raises if featurize() hasn't run yet. FeaturizerBase no longer defines feature_blocks at all.
for more information, see https://pre-commit.ci
FeaturizerEntry only ever normalized a list-form featurizer entry inside validate_featurizers and was never constructed or imported anywhere else, so a full BaseModel with its own validator and extra="forbid" config was more machinery than the two-branch dict-shape check needed. The dropped extra="forbid" check wasn't protecting much either: FeaturizerBase subclasses don't set extra="forbid" themselves, so a typo inside params was already silently ignored, same as a typo would now be at the wrapper level.
hasattr(self.feat, 'feature_blocks') was the only capability check in the codebase testing for method presence instead of a declared ClassVar[bool] flag; accepts_feature_blocks, pickleable, and is_cross_val all follow the declared-flag pattern instead. provides_feature_blocks brings this check in line with that convention: False on FeaturizerBase, True on FeatureConcatenator, the only current producer.
…olled parsing FeatureConcatenator was the only place in anvil that reimplemented the type-string-to-class resolution that AnvilSection.to_class() already provides, and that hand-rolled path had accumulated a third accepted entry shape (single-key dicts) that no config actually used. List entries now resolve via FeatureSpec.to_class(), and the single-key shape is dropped. FeatureSpec runs with pydantic's default extra=ignore, so an entry missing 'type' would parse as type=None and fail later with a confusing message; a guard rejects it up front instead. The whole-field dict form is kept but now warns, since saved recipe YAMLs still use it and cannot be rewritten retroactively.
The test asserted that a nested entry inherits AnvilSection's alias mapping, using a featurizer that declares no random_seed field. That behavior is incidental to the entry-resolution contract this module covers, and warning toward a field the class does not define is a defect rather than something to pin. Entry resolution through FeatureSpec is already covered by test_concatenator_list_entry_forms.
The Featurization section still taught the whole-field dict form for
`FeatureConcatenator.featurizers`, which now emits a DeprecationWarning,
while the new Transform section used the supported `{type, params}` wrapper
list. Switch the concatenator example to the wrapper form so both sections
agree, and note that entries take the same shape as the `feat` section
itself.
Transforms were described as row-wise; PCA reduces columns and imputation
fills from per-column statistics, so drop the axis claim in both
`anvil_reference.rst` and `_api/api/transforms/index.rst`. Fix the "depth
learning" typo and add the `constant` strategy missing from the
`ImputeTransform` row.
Every `code-block:: yaml` directive in `anvil_reference.rst` was missing the
blank line that separates a directive from its content, so docutils rejected
the argument list and rendered an error in place of the YAML. Add it to all
eighteen.
Three parse errors, each of which dropped content rather than merely rendering it oddly. `**Parameters**` sat directly against `.. list-table::` in the metadata and data sections, so docutils read the table as an indented block under the paragraph and rejected it. The `train_resource`, `val_resource`, and `test_resource` rows in the data table were indented one space deeper than the rest of the table, and their description continuation lines deeper still, so the three rows were not part of the table at all. The report section's `Example` heading used the level-5 underline that the rest of the file reserves for subsections of `~` headings, skipping a level below the `^` of `Report`.
Both are unconditional third-party dependencies with no import cycle to break, and the module already imports torch and lightning at top, so deferring them to the transform-persistence block saved nothing.
FeatureSpec.to_class() unwraps to `get_featurizer_class(type)(**params)` plus the AnvilSection validators, of which only the deprecated `random_state` alias has any effect. Calling the registry directly says the same thing in one step, drops the function-local spec import that existed only to avoid a cycle, and makes both the list and deprecated-dict branches resolve the same way. The cost is that a nested entry carrying `random_state` loses its seed rather than mapping it. Both featurizers declaring `random_seed` (ChemPropFeaturizer, PairwiseFeaturizer) emit DataLoaders and cannot be concatenated, so the gap is unreachable in a configuration that otherwise works, and #596 closes it by moving the alias onto the classes.
A bare `TypeName:` with nothing after it parses as None, not an empty mapping, so splatting it raised TypeError. The list form already guards this; apply the same treatment to the dict form.
Leaving None in the type kept an unseeded, unreproducible fit reachable, which is not a mode worth offering on a transform whose loadings are persisted and re-applied at inference. The field is now int, so every fit is reproducible by construction. Nothing passed None: no recipe sets a null seed and no caller relied on the optional form.
This reverts commit a2d2833.
The missing/unexpected comparison was written out twice, once at workflow construction against the featurizer's declared blocks and once inside PCATransform.fit as the backstop for transforms fitted outside a workflow. Both said the same thing in slightly different words, so hoist the comparison and its error into check_block_keys in transform_base and have each caller name its own subject: 'n_components' from the transform, the transform's class name from the workflow.
Whether a transform uses feature blocks was answered in two places: a hand-maintained ClassVar declaring that fit takes the kwarg, and required_block_keys reporting the keys the instance is configured against. Nothing kept the two in agreement, and a transform that needed the layout but forgot the flag was silently never given it, fitting over the whole matrix instead. fit_transforms is the only caller of a transform's fit, so it now passes feature_blocks unconditionally and transforms that ignore the layout absorb the kwarg. A transform whose fit cannot accept it now fails loudly instead. required_block_keys is left as the single answer to whether an instance is configured per block.
The fit-time guard rejected n_components at min(train rows, block width), one stricter than sklearn's own bound, which admits a full-rank PCA. A four-column block could therefore ask for at most three components, losing a dimension to a check rather than to a modeling decision.
Per-block n_components required a count for every block, so a block that wanted no reduction had only a full-rank entry, which centers and rotates the columns into linear combinations. That is the wrong shape for a narrow block whose columns each mean something, such as the per-task predictions of a pretrained model used as features. A null value records the block with no pipeline and slices it straight into the output at fit and transform alike. Every block still needs an entry, so passthrough stays explicit and check_block_keys still rejects a mistyped block name.
dwwest
reviewed
Sep 2, 2026
dwwest
left a comment
dwwest
left a comment
Contributor
There was a problem hiding this comment.
I need to chat about the blocks again I think, I'm not sure I totally understand that.
dwwest
approved these changes
Sep 4, 2026
dwwest
left a comment
dwwest
left a comment
Contributor
There was a problem hiding this comment.
Looks good, I think mostly comments to make things clearer. Example use-cases would really help
ProcedureSpec and WorkflowBase both accept either one transform or an ordered list, and each carried its own byte-identical validator body to normalize the single form. Move that into anvil/utils.ensure_list so the rule lives in one place. The helper goes in a new module rather than either existing file because workflow_base imports specification while specification defers an import back from workflow; a neutral module keeps the extraction out of that cycle. DataSpec.check_target_cols_input is deliberately left alone. It wraps only str and passes every other value through for pydantic to reject, which is not the same rule.
Six comment blocks added by this branch ran to three lines each, restating what the code below them already showed. Cut each to the part that is not readable from the code, and drop the trailing period on the width guard.
Four comments added by this branch sat as the first line inside an if or else body while describing the decision that selected the branch. Move each above its gate, and drop the one in the deprecated-dict path that only restated the warnings.warn call below it. The 1D note in predict stays inside the branch, since it explains the atleast_2d call rather than the decision to transform at all.
The class docstring described the per-block mechanism without ever saying what it buys. A single PCA over a concatenated matrix spends its components on whichever block carries the most raw variance, so a 2048-bit fingerprint crowds a couple of hundred descriptors out of the retained dimensions; per-block fitting gives each featurizer its own budget. _check_2d likewise only restated its own condition. Record what it catches: featurizers emit 1D for single-row input, which block slicing would meet with a bare IndexError.
The docstring said the sort had moved to an after-validator without saying why anything is sorted at all, which is the question a reader arrives with. Record that the emitted column order is part of the featurizer's contract, that deriving it from the featurizer set keeps two recipes naming the same featurizers interchangeable, and that per-block transforms depend on it because they apply their pipelines in emit order.
The docstring stated the rule abstractly. Add the case a reader hits: a PCATransform naming only FingerprintFeaturizer against a concatenator that emits a descriptor block too, why that is rejected, and the two ways to satisfy it, including a null count for passthrough.
The section stated the rule and showed a recipe without ever saying what comes out the other side. Give the concrete widths for the recipe above it, 223 descriptor columns and 2000 fingerprint columns reducing to 288, and name the emitted column order. That ordering was undocumented anywhere and is genuinely surprising: blocks emit in featurizer class-name order, so the descriptor block precedes the fingerprint block even though the recipe writes the fingerprint first. Also drops the doubled featurized/featurization in the section opener.
Second pass over the same threads. The prose had drifted into narrating rationale where a label was wanted, so cut the comments to what they name and drop the history from the sort docstring in favour of current state. Restores a deprecation label on the dict path, this time above the gate rather than inside it, and states the same-class featurizer limit in the per-block PCA docs, since keying blocks by class name is what causes it.
Both docstrings buried the operational modes in a running paragraph. Break them into labeled entries: global, per-block, and passthrough for the transform itself, and global versus per-block plus the one-to-one key contract for the workflow check.
The file was named for a concept rather than a module, and there is no transforms/transform_sequence.py. Everything it exercises, to_transform_list, fit_transforms, and transform_features, lives in transform_base, so the name now matches the one-test-file-per-module layout the suite follows elsewhere.
transforms/impute.py had no test file, so its only coverage was incidental, as a supporting component inside the sequence tests. Cover the strategies against hand-computed column statistics, that fitted statistics are reused on a later batch rather than recomputed, the unfitted guard, seeded reproducibility for the iterative imputer, and construction-time rejection of an unknown strategy or imputer. Includes a direct regression test for fit returning self. The chained calls in the sequence tests already depended on it, but nothing named it.
anvil/utils.py arrived without a test module. Pin the three branches plus the cases the wrapper exists to get right: a mapping counts as one entry rather than an iterable of keys, and falsy values are wrapped rather than treated as absent.
smcolby
commented
Sep 8, 2026
make_intake_cat.ipynb arrived with the first intake ingestion work and was never runnable: it passes a string where Catalog wants a dict of user parameters, so two cells carry committed AttributeError tracebacks and the cell that would write example_intake.yaml sits below them. Nothing imports or executes it, and the tracebacks embed a contributor's local conda path. The catalog it was meant to generate is committed separately and stays. datafiles.py exposes example_intake.yaml as intake_cat and eight recipe YAMLs resolve it through ANVIL_DIR. Removing it also takes the file out of ruff's notebook formatting, which is what pulled it into this branch in the first place.
The entry ran six lines and was the longest random_seed description in the repo, while being the only one that omitted the deprecated-alias sentence every other one carries. TransformSpec subclasses AnvilSection, so PCATransform does accept random_state in recipe params like any other section. Adopt the wording used by split_base, tabpfn, cross_validation, and impute. The default and the meaning of None are already visible in the signature, and the sklearn randomized-solver detail describes PCA rather than this field.
smcolby
added a commit
that referenced
this pull request
Sep 10, 2026
conftest.py conflicted only by adjacency: main appended chemeleon_foundation_checkpoint where this branch appended fingerprint_model_dir. Both fixtures are kept. The rest was a silent API break rather than a textual conflict. #592 gave load_anvil_model_and_metadata a fitted-transform return, so the featurizer's four-way unpack raised, and a pretrained model trained in transform space would have been predicted on with raw features. TrainedModelFeaturizer now caches and applies that transform, reproducing the pretrained model's inference path in full, guarded by transform_model_dir and a test that fails when the transform is skipped. #592 also deprecated the whole-field dict form for FeatureConcatenator and dropped its class-name sort, so the concatenator test moves to the list form and its comment now describes configured order.
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
3 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.
Description
Adds PCA dimensionality reduction as a first-class step in the anvil pipeline.
PCATransformoccupies a newtransformslot in the procedure section, between featurization and training. With an integern_componentsit runs a single PCA over the full feature matrix; with a dict it reduces each featurizer block to its own component count (e.g. fingerprints to 256 dims and descriptors to 32), resolving the block layout from the featurizer via a newfeature_blocksprotocol. A per-block count ofnullpasses that block through unreduced, for blocks whose columns carry meaning one by one and would only be rotated by a PCA; every block still needs an entry, so passthrough is explicit and a mistyped key is still caught. An optionalimpute_strategyhandles NaNs ahead of the reduction since PCA cannot see NaNs itself.Fitting happens on the train partition only, the transform is applied to every partition, and the fitted transform is persisted beside the model (schema-versioned
transform.pickle) so that decoupled inference sees exactly the features training saw:predict()loads the artifact and applies it, and raises loudly instead of silently skipping when a model trained with a transform has no artifact. Thetransformsection accepts a single transform or an ordered list, so column operations such as median imputation followed by per-block PCA can be stacked. Transforms are consumed by tabular-input workflows only: the sklearn workflow rejects featurizers that emit DataLoaders, and the lightning workflow rejects transforms, so ChemProp pairings fail at construction with a clear message.The transform and its block protocol
PCATransformin the transforms registry: int or per-blockn_components(an int, ornullfor passthrough), optional mean/median imputation, per-entryrandom_seed; validates block keys against the featurizer layout, exact width coverage of the raw matrix, and component counts against the fitting data, where a count may go up to the block rank as sklearn itself allowsfit_transforms/transform_featureshelpers to fit and apply a single transform or an ordered sequence, threading each output into the nextFeaturizerBase.feature_blocksreporting the raw featurizer's column layout keyed by registry name;FeatureConcatenatorflattens nested concatenations in emit orderTraining and inference wiring
transformaccepts aTransformSpecor a list of them; the global seed fills entries that set noneload_anvil_model_and_metadatareturns the fitted transform as a fifth element (existing in-package callers updated);predict()applies it after featurization, normalizes 1D single-row output, and checks that row count is preservedFeatureConcatenatoraccepts the list entry form used in recipe YAML and rejects duplicate featurizer classes, whose identical block keys could not be disambiguatedImputeTransform.fitreturnsselfas its docstring always claimed (chained usage was previously broken)Tests and docs
transformsection (single and per-block examples) in the anvil referenceNotes for reviewers
transform.pickleand will now hard-error at inference until retrained (previously inference would have consumed the wrong features silently).Quality Assurance & AI Policy
To maintain project quality and respect maintainer bandwidth, please confirm the following:
Status
Developers Certificate of Origin
Note to Contributors: We reserve the right to close PRs without review if they appear to lack human validation or do not meet the quality standards described in our
CONTRIBUTING.md.