Hypothesis testing not consistently implemented for "Multidetector" analysis? #584
Comments
|
I'm reworking things as a part of my upcoming PR and would be grateful for some input here. |
|
You'r right, the code in L900 and L908 should be similar to L739 |
thehrh
pushed a commit
to thehrh/pisa-1
that referenced
this issue
Mar 19, 2020
philippeller
added a commit
to philippeller/pisa
that referenced
this issue
Sep 29, 2020
* changes include: * create some small common statistical routines in `utils.stats` * create routines for pull method in `utils.pull_method` ** unit test ok * create fisher matrix routine in `utils.fisher_matrix` ** needs tidying up * create `utils.minimization` module holding tools and constants for minimization tasks ** moved over from `analysis.py`: *** `MINIMIZERS_USING_SYMM_GRAD`: unchanged *** `set_minimizer_defaults`: add basinhopping, simply structure of `minimizer_settings` dict *** `validate_minimizer_settings`: add basinhopping *** `Counter`: make `__iadd__` return instance ** add `_run_minimizer` function *** wrapper around local and global minimizer routines ** add `_run_local_minimizer` function ** add `_run_global_minimizer` function * `analysis.analysis`: ** `check_t23_octant` -> `t23_octant` ** `Analysis` class: *** add `_calculate_metric_val` static function *** add `optimize_discrete_selections` function *** add `fit_from_startpoints` function *** add `optimize_t23_octant` function *** `fit_hypo`: **** moved the theta23 octant checking logic into `optimize_t23_octant` function **** removed check_ordering functionality, have the more general optimize_discrete_selections for that *** rename `fit_hypo_inner` -> `_fit_hypo_inner` to clarify that it shouldn't be called by the user **** dispatch the different fit routines depending on fit settings *** implement `_fit_hypo_minimizer` **** basically what `_fit_hypo_inner` was doing **** we are calculating the sign with which the metric value has to be multiplied anyway, so we can correct for it upon reporting: pass it into minimizer callable instead of letting the latter reevaluate it over and over (also: performance) **** calls `_run_minimizer` for the core logic *** implement `_fit_hypo_pull` **** runs the pull method * * `utils.config_parser`: ** add method `parse_minimizer_config` ** add method `parse_fit_config` ** dummy unit tests * TODO: fix nofit_hypo usage, update scripts/analysis.py * `analysis.hypo_testing`: ** add methods `validate_maker_names`, `setup_makers_from_pipelines`, `collect_maker_selections`, `select_maker_params` *** ensure consistent state of initialisation arguments for hypothesis testing ** `Labels` class: *** add `fluctuate_data_method` and `fluctuate_fid_method` attributes ** `HypoTesting` class: *** introduce some further initialization parameters **** reflecting fluctuation methods, also for randomization of fit starting points, for optimisation over discrete hypos, for forcing fits to be performed even though we know what the best-fit will be a priori **** many minor fixes **** add `hypo_scan` function * `core.map`: ** `Map` class: *** missing `random_state` now also used by methods != "poisson" *** add function `signed_sqrt_mod_chi2` * `utils.stats`: ** added function `signed_sqrt_mod_chi2` * `core.prior`: ** in `get_prior_bounds`: *** some small fixes (accept a `Prior` instance) * `scripts.discrete_hypo_test`: ** replaced the setting up init args with helper from `analysis.hypo_testing` * `scripts.inj_param_scan`: ** cf. above * `scripts.profile_scan`: ** using the `HypoTesting` class now * create `scripts.simple_fit`: ** for someone who really just wants to run a simple fit * `scripts.systematics_tests`: ** cf. above * pisa examples: ** some minimizer configs in the new format * fixed bug in example minimizer config * fixed missing import in fitting.py * small fixes, e.g. icecube#584 * * `scripts.analysis`: ** class `AnalysisScript` created to replace the unwieldy maintenace of command line parser arguments for the various analysis scripts *** shared parsers to which every script has access and which can be combined as desired *** automatic validation of arguments, where desired *** central function for creating valid `init_args_d` dictionary to be passed to the various analysis functions * unit test for parsing of fit and minimiser config, add example fit config * replaced other minimizer configs * fix when is passed; also implement some testing functionality for * unit test for local minimization routine and small fix in generating metric info * tests for local and global minimization * getting there * necessary fixes, documentation, test improvements * mostly documentation * minor cleanups * define fluctuate methods centrally; some fixes from testing discrete hypo test * fix order of type checks in parsing methods * adapt setup.py for installation of minimizer and fit configs with .cfg format * fix up validation of minimization settings and make unit test more robust (use ftype minimizer defaults) * avoid modifying minimizer and fit settings in-place, introduce separate-octant fitting to hypo testing, minor fixes * also fix basinhopping disp option * reintroducing nelder-mead, along with some minor mods * revert get_random_state; fixme note * now have to pass random state value for randomisation of parameter values; revert to old behaviour of fitting with priors in syst tests * minor fixes to simple fit to reflect the fact that it doesn't use HypoTesting * found some bugs and fixed them; be very explicit in what's happening Co-authored-by: Thomas Ehrhardt <tehrhard@uni-mainz.de>
|
Closing because |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Look at how we produce pseudodata by looping over the sequence of asimov distributions for each experiment/detector, https://github.com/IceCubeOpenSource/pisa/blob/master/pisa/analysis/hypo_testing.py#L739, vs. how we don't for pseudodata of the "fiducial" hypothesis: https://github.com/IceCubeOpenSource/pisa/blob/master/pisa/analysis/hypo_testing.py#L900 and https://github.com/IceCubeOpenSource/pisa/blob/master/pisa/analysis/hypo_testing.py#L908.
Hard for me to believe this works. Has this been tested? I would have expected the change to have been part of #505 @JanWeldert
The text was updated successfully, but these errors were encountered: