Repository navigation
Restructure cd into the package architecture (PR A of 4) - #2
Merged
Merged
Conversation
whitespace=cr-at-eol only silenced diff warnings; -text stops git from normalizing the CRLF line endings on any checkout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PanelInputs and SCSPILLEffects move to a leaf module, containers.py, and the sar inputs, posterior blocks and fit move to sar/structures.py (SARInputs, AlphaPosterior, SARPosterior, SARFit). structures.py keeps re-exporting every old name, and SCSPILLInputs/SCSPILLFit stay as aliases. Panel ingestion becomes model-agnostic: prepare_panel_inputs builds the treated/control split for any model, now also rejecting duplicate unit-period rows and a non-0/1 treatment indicator, and optionally accepting several treated units with a common start. The sar model adds its spatial weights in sar/setup.py; prepare_scspill_inputs remains as the composition of the two. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One configuration class for every model: method becomes Literal['sar', 'cd'] and cd's fields (affected_units, spillover_structure, effect_matrix, reference_mode, quantile_method, ci_method, scm_maxiter, scm_ftol) join SCSPILLConfig with a (method='cd') prefix; sar's fields get (method='sar'). CDConfig is gone. - spatial_w and spatial_W become optional and are required only for sar; the burn < m_iter and covariate checks apply to sar only. - Setting a field of the model that is not selected is an error that names the fields, instead of being silently ignored. - A custom exposure matrix is selected with spillover_structure='custom'. Each treated unit's row must be its own direct-effect indicator, so a spillover column can no longer leak into the headline effect. - affected_units may no longer list the treated unit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
quantile_method now defaults to 'empirical' (the left inverse of the empirical CDF) and ci_method to 'inversion' (inverting the squared P test), with full-sample references, as Cao & Dowd v3 Section 4 defines them. This follows the sar model's rule: paper-correct defaults, with the reference implementation's behaviour kept as switches. quantile_method='linear' and ci_method='published' still reproduce the authors' Proposition 99 application code, and the tests against their frozen output now pass those settings explicitly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every loader now returns the same type. SpillPanel's spatial weights become optional and it gains affected_units, the units published as potentially exposed. config_kwargs(method=None) configures sar when the panel has spatial weights and cd otherwise, and either model can be asked for explicitly; the sar keyword set is unchanged. CDPanel is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SCSPILLResults gains a sar: SARFit slot, the first of one slot per model. alpha_posterior, sar_posterior and mcmc_summary_table become properties that read it and raise AttributeError on a result from another model, so every v0.2 accessor keeps working. Assembling a sar fit into the standard result moves from the estimator to sar/report.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SCSPILLEffects becomes keyword-only. The sar-only extras (att_draws, att_plugin, propagate_alpha, n_draws_used) are optional, att_ci may be None, and interval_kind records whether the bands are posterior credible bands or Cao-Dowd confidence intervals; plot legends follow it. The plotter keeps a per-model table of supported kinds, and the weights panel reads the standard donor_weights with a model-specific comparator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The flat cd.py becomes scspill_helpers/cd/, laid out like sar/ and mlsynth's SPILLSYNTH cd/: - setup.py: the exposure matrix and prepare_cd_inputs - scm_core.py: the intercept-adjusted simplex SCM equations - estimation.py: the effect maps - inference.py: P tests, intervals, kappa_A - pipeline.py: run_cd - report.py: maps the fit onto the standard result - structures.py: CDInputs, CDTestResult, CDFit The numerics are unchanged: the tests against the authors' frozen output pass as before. fit() now runs the same data, estimation and plotting blocks, with the same exception translation, for both models. A cd fit returns an SCSPILLResults with method='cd' and the fit in its cd slot, so CDResults is gone. The shared surface (att, counterfactual, donor_weights, plot) works for both models; cd-only accessors (effects_panel, effect_ci, pointwise_p_values, kappa_test, test_hypothesis, ...) raise AttributeError on a sar result, and vice versa. cd fills the shared effect contract: the counterfactual band is the selected pointwise interval after treatment, and the spillover panel holds each control's SCM residuals before treatment and its estimated spillover after, as sar does. That makes the panel, full, effect, spill_top and weights plots work for cd. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
'spillover' draws the direct effect and every exposed control's spillover by post period, with the selected pointwise interval; 'kappa' draws the kappa_A structure statistic against its critical value and the pre-treatment reference range. Asking a model for a kind it does not support names the model in the error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Cite the paper's v3 throughout. State that cd's defaults are the paper's Section 4 definitions, with the application code behind switches, that a custom effect_matrix needs spillover_structure='custom', and that a fit returns SCSPILLResults. The full rewrite of the model page comes with the documentation PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
Folds the
cdmodel merged in #1 into the architecturesaruses, with no new estimators. This is the first of four stacked PRs: (A) this restructure, (B) newcdfeatures, (C) documentation and website, (D) the v0.3.0 release.One config, one result type
cdis selected withSCSPILLConfig(method="cd"). Its fields are prefixed(method='cd'), andsar's are prefixed(method='sar').CDConfigis removed.ScspillConfigError, and the message names the field.fit()returnsSCSPILLResultsfor both models. The model's own fit sits in a typed slot,result.sarorresult.cd.CDResultsis removed.att,counterfactual,gap,donor_weights,effects_detailandplot().AttributeErroron the other. Every v0.2saraccessor still works.Per-model subpackages, as in mlsynth's SPILLSYNTH
scspill_helpers/cd/containssetup,scm_core,estimation,inference,pipeline,reportandstructures.sarcontainers move tosar/structures.py, andsar/report.pyassembles thesarresult.containers.pyholds the sharedPanelInputsandSCSPILLEffects.SCSPILLInputsandSCSPILLFitremain as aliases.prepare_panel_inputs), andfit()runs the same data, estimation and plotting blocks for both models, with the same exception translation.Defaults follow the paper (Cao & Dowd v3, Section 4)
quantile_method="empirical"(left inverse of the empirical CDF),ci_method="inversion", andreference_mode="full".quantile_method="linear"andci_method="published". The tests against their frozen output pass these settings explicitly.Smaller fixes
effect_matrixnow requiresspillover_structure="custom". Each treated unit's row must be its own direct-effect indicator.affected_unitsmay not list the treated unit.load_california_cdreturns aSpillPanel. Spatial weights are now optional,affected_unitsis new, andconfig_kwargs(method=None)infers the model.CDPanelis removed..gitattributesuses-text, so the authors' state list stays byte-identical.cdplots:panel,full,effect,spill_topandweightsnow work forcd. Two new kinds,spilloverandkappa.sar.Behaviour change for
sar. Shared ingestion now also rejects duplicate unit-period rows and a treatment indicator that isn't 0/1. This is noted in the CHANGELOG.Validation
make lint,make typecheckandmake test: 248 passed, 1 skipped.pytest -m slow: 4 passed, 1 skipped.test_cd.pyassertions are unchanged except imports and accessor paths. With the application-code settings, P values still match the authors' frozen output exactly, and effects match within 0.03.sarresults for a fixed seed (California and Sudan, numpy backend) match the pre-refactor run within the macOS Accelerate float noise. The rho chains agree to 1e-12.quartodoc buildsucceeds with the moved symbols.Docs carry interim truth fixes only: v3 citations, the new defaults, and
SCSPILLResults. The full documentation lands in PR C.🤖 Generated with Claude Code