Polish analysis of a multiclosure test

[comane]

Here we collect new functions and template that allow the analysis of multiclosure tests. @comane @giovannidecrescenzo @mariaubiali @andreab1997

[andreab1997]

@comane thanks for this. If you want, ask myself for the review (but also add one between @scarlehoff and @RoyStegeman given that part of this PR is based on my work and it does not make sense for me to review my own functions )

[scarlehoff]

This is then ready to be merged? (@andreab1997 @comane )

[andreab1997]

This is then ready to be merged? (@andreab1997 @comane )

I don't think so, surely I need to review this again but in any case I would wait for the paper to be published.

[scarlehoff]

This piece is complete and already rebased on top of master isn't it? If so it should be merged, worst case scenario you can note down the checksum of the commit but leaving it as a branch risks nobody taking care of it after the paper is out (which is what happened with the previous closure test branches and basically meant redoing a lot of stuff that mwilson already did)

[andreab1997]

This piece is complete and already rebased on top of master isn't it? If so it should be merged, worst case scenario you can note down the checksum of the commit but leaving it as a branch risks nobody taking care of it after the paper is out (which is what happened with the previous closure test branches and basically meant redoing a lot of stuff that mwilson already did)

Ok I agree but still I would like to have another look. I will do before the end of this week in such a way we can merge this before saturday. Is that ok?

[comane]

Hi @andreab1997, I addressed most of the comments that you left, as well as those that I wrote myself.

There is one main issue with this PR at present, namely, the way in which the compare inconsistent closure test script works.

This script is problematic, because it's not possible to have a dataset_inputs which is different from from_: fit, note that this is a problem since we want to have out of sample datasets.

The problem, I think, can be solved by removing the vp-compare fits from compareinconsistentclosuretemplates. We don't need to have another vp-comparefits anyways.

If you could take care of this that would be great.

[andreab1997]

Hi @andreab1997, I addressed most of the comments that you left, as well as those that I wrote myself.

There is one main issue with this PR at present, namely, the way in which the compare inconsistent closure test script works.

This script is problematic, because it's not possible to have a dataset_inputs which is different from from_: fit, note that this is a problem since we want to have out of sample datasets.

The problem, I think, can be solved by removing the vp-compare fits from compareinconsistentclosuretemplates. We don't need to have another vp-comparefits anyways.

If you could take care of this that would be great.

Just to understand, this issue is only there if you use the CLI or even if you write your own runcard and template?

[comane]

Just to understand, this issue is only there if you use the CLI or even if you write your own runcard and template?

I think it's there if I run validphys multiclosure_analysis.yaml (with dataset inputs that is not from the fit)

[andreab1997]

Just to understand, this issue is only there if you use the CLI or even if you write your own runcard and template?

I think it's there if I run validphys multiclosure_analysis.yaml (with dataset inputs that is not from the fit)

Ok I need to test this, maybe it is something fixable

[comane]

@scarlehoff, once the tests pass, this is ready to merge in my opinion

[comane]

The only general question I have is, why do you need so many extra functionality under inconsisent_closuretest? I would have naively thought that many of the estimators could also be applied to the usual (multi)CT ?

I agree with you, the reason to do this is that the multiclosure.py module is already very large and full of unused functions. I have now put all of the general multiclosure functions in multiclosure.py. In a future PR I will then polish the multiclosure module from the old and unused multiclosure functions.