Repository navigation
Model: add opt-in summary-statistics schema validation - #678
lassekorvall67-cyber wants to merge 4 commits into
Conversation
|
Hello, thank you for your PR. As this PR seems heavily AI generated, could you please give us some more background on why this should be added to |
|
Thank you for raising this. Yes, this was developed with substantial ChatGPT/Codex assistance, as stated in the PR. The motivation was the consistency-check request in #470. One concrete example is observed The proposed benefit is an early error identifying the affected statistic, before flattening loses those boundaries. It is opt-in because variable-length statistics can be intentional. I do not have a production research case to add; the evidence here is a synthetic reproducer and tests. I also recognise that this narrower opt-in wrapper does not implement the default-with-override approach described in #470. I should have discussed the intended integration before proposing a new public class. Would a smaller example or a check at an existing extension point fit your plans better? |
|
Thank you. Given the limited practical use case and the additional maintenance this would introduce, I don’t think this is worth the effort at this point. |
Related to #470; this restricted helper does not close the whole issue.
Summary dictionaries can have the same flattened length even when their
per-statistic shapes differ. For example, observed a=[1,2], b=[3] and simulated
a=[1], b=[2,3] both flatten to [1,2,3], so the existing PNormDistance can return
zero. Its existing post-transformation shape check remains useful but cannot
detect the lost per-key boundaries.
CheckedSummaryStatistics(summary_statistics, reference)is an optionalmodule-level callable for
ABCSMC(summary_statistics=...). It snapshots thereference keys/shapes, calls the wrapped function once and validates the result
before distance transformations. It returns the same dictionary unchanged.
The reference must already be summarized and match the observed data supplied
to ABCSMC; schema changes require a new wrapper.
This first version supports string-keyed dictionaries with numeric scalars and
plain NumPy arrays (dtype kinds i/u/f/c), without dtype equality or a new finite-
value policy. It rejects unsupported values explicitly. Scalar/0D shape () is
different from (1,). Missing/extra keys and shapes produce contextual errors.
It is exported through pyabc.model and top-level pyabc. No existing defaults,
distance implementations, storage formats or upstream tests are changed.
Schema errors stop the ordinary run with an exception; they do not merely
reject a particle and continue sampling.
Custom models must invoke this callable to use its check. IntegratedModel
results are not validated automatically. Variable-length applications can keep
their existing behavior by not selecting the wrapper. A default check for all
model paths, pandas-label semantics and broader value-type support remain
outside this change and need maintainer discussion.
How to test
With this change applied and the project's ot extra installed for the existing
distance tests:
python -m pip install -e '.[ot]' pytest==9.1.1 python -m pytest test/base/test_summary_statistics_check.py -q python -m pytest test/base/test_summary_statistics_check.py test/base/test_distance.py test/base/test_sumstat.py test/base/test_integrated_model.py -qRecorded latest selected result: 106 passed, 162 warnings, comprising 71
new cases and 35 existing tests. The warnings are from existing multiprocessing
fork paths. An earlier stochastic
test_sumstat.test_pipelineordering assertionfailed in both the modified checkout and an unchanged base; those failed logs
are retained. No threshold was relaxed. The new tests preserve the callers'
NumPy/Python RNG states. The full suite was not run.
New checks cover the original per-key mismatch, supported/unsupported types,
metadata, no coercion/copying, reference mutation, scalar/0D distinctions,
NumPy timedelta scalar regression, exception propagation, pickle/cloudpickle
and ordinary FunctionModel/SingleCore ABCSMC paths. A separate runnable demo
also compares identical valid inferences with/without validation, including
initial epsilon calibration, and rejects malformed calibration data.
Ruff 0.17.2 lint/format checks pass. The added RST guide parses without warnings
and its full usage example was executed. Full Sphinx, other operating systems
and all distributed backends were not tested. The synthetic examples do not
establish biological validation or a speed improvement.
Developed by Lars Körvall with ChatGPT/Codex assistance. Feedback on the
opt-in API and appropriate future integration is welcome before acceptance.
Local environment: Linux, Python 3.12.14, NumPy 2.5.4, SciPy 1.18.1 and pytest 9.1.1. GitHub CI and maintainer review are pending.