Skip to content

Commit 76d430b

Browse files
lukmazThe Meridian Authors
authored andcommitted
Remove customization of quality checks.
PiperOrigin-RevId: 865047044
1 parent d70b2b0 commit 76d430b

File tree

2 files changed

+25
-48
lines changed

2 files changed

+25
-48
lines changed

meridian/analysis/review/reviewer.py

Lines changed: 4 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
ConfigInstance = configs.BaseConfig
3030
ChecksBattery = immutabledict.immutabledict[CheckType, ConfigInstance]
3131

32-
_DEFAULT_POST_CONVERGENCE_CHECKS: ChecksBattery = immutabledict.immutabledict({
32+
_POST_CONVERGENCE_CHECKS: ChecksBattery = immutabledict.immutabledict({
3333
checks.BaselineCheck: configs.BaselineConfig(),
3434
checks.BayesianPPPCheck: configs.BayesianPPPConfig(),
3535
checks.GoodnessOfFitCheck: configs.GoodnessOfFitConfig(),
@@ -39,47 +39,29 @@
3939

4040

4141
class ModelReviewer:
42-
"""Executes a series of quality checks on a Meridian model.
42+
"""A tool for executing a series of quality checks on a Meridian model.
4343
4444
The reviewer first runs a convergence check. If the model has converged, it
4545
proceeds to run a battery of post-convergence checks.
4646
47-
The default battery of post-convergence checks includes:
47+
The battery of post-convergence checks includes:
4848
- BaselineCheck
4949
- BayesianPPPCheck
5050
- GoodnessOfFitCheck
5151
- PriorPosteriorShiftCheck
5252
- ROIConsistencyCheck
53-
Each with its default configuration.
54-
55-
This battery of checks can be customized by passing a dictionary to the
56-
`post_convergence_checks` argument of the constructor, mapping check
57-
classes to their configuration instances. For example, to run only the
58-
BaselineCheck with a non-default configuration:
59-
60-
```python
61-
my_checks = {
62-
checks.BaselineCheck: configs.BaselineConfig(
63-
negative_baseline_prob_review_threshold=0.1,
64-
negative_baseline_prob_fail_threshold=0.5,
65-
)
66-
}
67-
reviewer = ModelReviewer(meridian_model, post_convergence_checks=my_checks)
68-
```
6953
"""
7054

7155
def __init__(
7256
self,
7357
meridian,
74-
post_convergence_checks: ChecksBattery = _DEFAULT_POST_CONVERGENCE_CHECKS,
7558
):
7659
self._meridian = meridian
7760
self._results: list[results.CheckResult] = []
7861
self._analyzer = analyzer_module.Analyzer(
7962
model_context=meridian.model_context,
8063
inference_data=meridian.inference_data,
8164
)
82-
self._post_convergence_checks = post_convergence_checks
8365

8466
def _run_and_handle(self, check_class, config):
8567
instance = check_class(self._meridian, self._analyzer, config) # pytype: disable=not-instantiable
@@ -139,7 +121,7 @@ def run(self) -> results.ReviewSummary:
139121
)
140122

141123
# Run all other checks in sequence.
142-
for check_class, config in self._post_convergence_checks.items():
124+
for check_class, config in _POST_CONVERGENCE_CHECKS.items():
143125
if (
144126
check_class == checks.PriorPosteriorShiftCheck
145127
and not self._uses_roi_priors()

meridian/analysis/review/reviewer_test.py

Lines changed: 21 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -140,13 +140,21 @@ def setUp(self):
140140
self._mock_pps_result.__class__ = results.PriorPosteriorShiftCheckResult
141141
self._mock_pps_check.run.return_value = self._mock_pps_result
142142

143-
self._default_post_convergence_checks = immutabledict.immutabledict({
144-
self._mock_baseline_check_cls: configs.BaselineConfig(),
145-
self._mock_bayesian_ppp_check_cls: configs.BayesianPPPConfig(),
146-
self._mock_gof_check_cls: configs.GoodnessOfFitConfig(),
147-
self._mock_pps_check_cls: configs.PriorPosteriorShiftConfig(),
148-
self._mock_roi_consistency_check_cls: configs.ROIConsistencyConfig(),
149-
})
143+
patcher = mock.patch.object(
144+
reviewer,
145+
'_POST_CONVERGENCE_CHECKS',
146+
new=immutabledict.immutabledict({
147+
self._mock_baseline_check_cls: configs.BaselineConfig(),
148+
self._mock_bayesian_ppp_check_cls: configs.BayesianPPPConfig(),
149+
self._mock_gof_check_cls: configs.GoodnessOfFitConfig(),
150+
self._mock_pps_check_cls: configs.PriorPosteriorShiftConfig(),
151+
self._mock_roi_consistency_check_cls: (
152+
configs.ROIConsistencyConfig()
153+
),
154+
}),
155+
)
156+
patcher.start()
157+
self.addCleanup(patcher.stop)
150158

151159
def test_run_all_pass(self):
152160
self._mock_convergence_result.case = results.ConvergenceCases.CONVERGED
@@ -160,7 +168,6 @@ def test_run_all_pass(self):
160168

161169
review = reviewer.ModelReviewer(
162170
meridian=self._meridian,
163-
post_convergence_checks=self._default_post_convergence_checks,
164171
)
165172
summary = review.run()
166173

@@ -195,7 +202,6 @@ def test_run_pass_with_roi_consistency_review(self):
195202

196203
review = reviewer.ModelReviewer(
197204
meridian=self._meridian,
198-
post_convergence_checks=self._default_post_convergence_checks,
199205
)
200206
summary = review.run()
201207

@@ -229,7 +235,6 @@ def test_run_pass_with_gof_review(self):
229235

230236
review = reviewer.ModelReviewer(
231237
meridian=self._meridian,
232-
post_convergence_checks=self._default_post_convergence_checks,
233238
)
234239
summary = review.run()
235240

@@ -265,7 +270,6 @@ def test_run_pass_with_pps_review(self):
265270

266271
review = reviewer.ModelReviewer(
267272
meridian=self._meridian,
268-
post_convergence_checks=self._default_post_convergence_checks,
269273
)
270274
summary = review.run()
271275

@@ -292,7 +296,6 @@ def test_run_fail_not_converged_skips_other_checks(self):
292296

293297
review = reviewer.ModelReviewer(
294298
meridian=self._meridian,
295-
post_convergence_checks=self._default_post_convergence_checks,
296299
)
297300
summary = review.run()
298301

@@ -324,7 +327,6 @@ def test_run_fail_not_fully_converged(self):
324327

325328
review = reviewer.ModelReviewer(
326329
meridian=self._meridian,
327-
post_convergence_checks=self._default_post_convergence_checks,
328330
)
329331
summary = review.run()
330332

@@ -364,7 +366,6 @@ def test_run_fail_with_reviews(self):
364366

365367
review = reviewer.ModelReviewer(
366368
meridian=self._meridian,
367-
post_convergence_checks=self._default_post_convergence_checks,
368369
)
369370
summary = review.run()
370371

@@ -403,7 +404,6 @@ def test_run_converged_with_fail_and_review(self):
403404

404405
review = reviewer.ModelReviewer(
405406
meridian=self._meridian,
406-
post_convergence_checks=self._default_post_convergence_checks,
407407
)
408408
summary = review.run()
409409

@@ -436,7 +436,6 @@ def test_run_fail_baseline(self):
436436

437437
review = reviewer.ModelReviewer(
438438
meridian=self._meridian,
439-
post_convergence_checks=self._default_post_convergence_checks,
440439
)
441440
summary = review.run()
442441

@@ -474,7 +473,6 @@ def test_run_pass_with_baseline_review(self):
474473

475474
review = reviewer.ModelReviewer(
476475
meridian=self._meridian,
477-
post_convergence_checks=self._default_post_convergence_checks,
478476
)
479477
summary = review.run()
480478

@@ -508,7 +506,6 @@ def test_run_skip_checks_with_custom_roi_priors(self):
508506

509507
review = reviewer.ModelReviewer(
510508
meridian=self._meridian,
511-
post_convergence_checks=self._default_post_convergence_checks,
512509
)
513510
summary = review.run()
514511

@@ -537,7 +534,6 @@ def test_run_skip_checks_with_non_roi_priors(self):
537534

538535
review = reviewer.ModelReviewer(
539536
meridian=self._meridian,
540-
post_convergence_checks=self._default_post_convergence_checks,
541537
)
542538
summary = review.run()
543539

@@ -566,7 +562,6 @@ def test_run_with_default_configs(self):
566562

567563
review = reviewer.ModelReviewer(
568564
meridian=self._meridian,
569-
post_convergence_checks=self._default_post_convergence_checks,
570565
)
571566
review.run()
572567

@@ -600,10 +595,12 @@ def test_run_with_custom_configs(self):
600595
prior_upper_quantile=0.95,
601596
),
602597
})
603-
review = reviewer.ModelReviewer(
604-
meridian=self._meridian, post_convergence_checks=custom_checks
605-
)
606-
summary = review.run()
598+
599+
with mock.patch.object(
600+
reviewer, '_POST_CONVERGENCE_CHECKS', new=custom_checks
601+
):
602+
review = reviewer.ModelReviewer(meridian=self._meridian)
603+
summary = review.run()
607604

608605
self._mock_convergence_check_cls.assert_called_once_with(
609606
mock.ANY, mock.ANY, configs.ConvergenceConfig()
@@ -644,7 +641,6 @@ def test_run_twice_clears_results(self):
644641

645642
review = reviewer.ModelReviewer(
646643
meridian=self._meridian,
647-
post_convergence_checks=self._default_post_convergence_checks,
648644
)
649645
summary1 = review.run()
650646
summary2 = review.run()
@@ -668,7 +664,6 @@ def test_checks_status(self):
668664

669665
review = reviewer.ModelReviewer(
670666
meridian=self._meridian,
671-
post_convergence_checks=self._default_post_convergence_checks,
672667
)
673668
summary = review.run()
674669

0 commit comments

Comments
 (0)