Introduce inconsistent fit closure test data #2180

comane · 2024-10-17T14:34:46Z

Reimplements #1682

This pull request introduces a new class, InconsistentCommonData, to handle inconsistencies within closure tests and includes several related changes across multiple files. The most important changes include the addition of the new class, updates to the configuration parsing, and new filtering methods for handling inconsistent closure data.

New Class and Methods:

validphys2/src/validphys/closuretest/inconsistent_closuretest/inconsistent_ct.py: Added InconsistentCommonData class with methods to introduce inconsistencies in closure tests, including systematic_errors property, select_systype_table_indices, rescale_systematics, process_commondata, and export_uncertainties.

Configuration Updates:

validphys2/src/validphys/config.py: Added parse_inconsistent_data_settings method to parse inconsistent data settings from the YAML file.
validphys2/src/validphys/config.py: Updated produce_filter_data to include inconsistent_fakedata parameter for filtering inconsistent closure data. [1] [2]

Filtering Methods:

validphys2/src/validphys/filters.py: Added filter_inconsistent_closure_data_by_experiment and _filter_inconsistent_closure_data methods to handle filtering of inconsistent closure data. [1] [2]

Testing:

validphys2/src/validphys/tests/test_inconsistent_ct.py: Added unit tests for InconsistentCommonData class methods, including tests for the getter and setter of systematic_errors, select_systype_table_indices, rescale_systematics, process_commondata, and export_uncertainties.

Benchmarking of Code

Test against old code to see whether we reproduce it, eg the same Ratio bias variance results.

Test 0.0

Test that an inconsistent ct fit (using the inconsistent closure test filter) with lambda = 1.0 gives the same results as a consistent closure test:

https://vp.nnpdf.science/VIdN_z0ETx6Ph0szzKLY0g==

Multiclosure Test

DIS multiclosure test with maximal lambda inconsistency parameter (lambda=0)

https://vp.nnpdf.science/idXftjIBTp6r2ufbFIhMaA==

Also note that for a single fit with the same L1 data

This is the type of agreement found between old and new branch for a DIS only inconsistent closure test with lambda = 0
https://vp.nnpdf.science/sH-C_csbTGCPCdEGFXj_1w==

Example of inconsistent closure test runcard:

inconsistent_data_settings:

  treatment_names: [MULT]
  names_uncertainties: [CORR, SPECIAL]

  inconsistent_datasets:
    - HERA_NC_318GEV_EM-SIGMARED
    - HERA_NC_251GEV_EP-SIGMARED
    - HERA_NC_300GEV_EP-SIGMARED
    - HERA_NC_318GEV_EP-SIGMARED

  sys_rescaling_factor: 0.0


closuretest:
  errorsize: 1.0
  fakedata: true
  inconsistent_fakedata: true
  fakenoise: true
  fakepdf: 210223-mw-000_fakepdf
  filterseed: 1
  printpdf4gen: false
  rancutmethod: 0
  rancutprob: 1.0
  rancuttrnval: false

datacuts:
  q2min: 3.49
  t0pdfset: 210223-mw-000_fakepdf
  use_cuts: internal
  w2min: 12.5

dataset_inputs:
- dataset: NMCPD_dw_ite
  frac: 0.75
- dataset: SLACP_dwsh
  frac: 0.75
- dataset: SLACD_dw_ite
  frac: 0.75
- dataset: BCDMSP_dwsh
  frac: 0.75
- dataset: BCDMSD_dw_ite
  frac: 0.75
- dataset: CHORUSNUPb_dw_ite
  frac: 0.75
- dataset: CHORUSNBPb_dw_ite
  frac: 0.75
- cfac:
  - MAS
  dataset: NTVNUDMNFe_dw_ite
  frac: 0.75
- dataset: HERACOMBNCEM
  frac: 0.75
- dataset: HERACOMBNCEP575
  frac: 0.75
- dataset: HERACOMBNCEP820
  frac: 0.75
- dataset: HERACOMBNCEP920
  frac: 0.75
- dataset: HERACOMBCCEP
  frac: 0.75
- dataset: HERACOMB_SIGMARED_C
  frac: 0.75

debug: false

description: DIS only data. Some datasets (see partition) are left out of the fit.
  Partition is chosen as in NNPDF40_hyperopt. An inconsistency of type 2 is introduced
  for some of the in-sample datasets, namely all of the HERA NC ones.

fitting:
  savepseudodata: true
  basis:
  - fl: sng
    largex:
    - 1.498
    - 3.138
    smallx:
    - 1.121
    - 1.154
    trainable: false
  - fl: g
    largex:
    - 3.266
    - 6.214
    smallx:
    - 0.9224
    - 1.149
    trainable: false
  - fl: v
    largex:
    - 1.6
    - 3.588
    smallx:
    - 0.5279
    - 0.8017
    trainable: false
  - fl: v3
    largex:
    - 1.761
    - 3.427
    smallx:
    - 0.2011
    - 0.4374
    trainable: false
  - fl: v8
    largex:
    - 1.589
    - 3.378
    smallx:
    - 0.5775
    - 0.8357
    trainable: false
  - fl: t3
    largex:
    - 1.763
    - 3.397
    smallx:
    - -0.484
    - 1.0
    trainable: false
  - fl: t8
    largex:
    - 1.572
    - 3.496
    smallx:
    - 0.6714
    - 0.9197
    trainable: false
  - fl: t15
    largex:
    - 1.503
    - 3.636
    smallx:
    - 1.073
    - 1.164
    trainable: false
  fitbasis: EVOL

genrep: true

integrability:
  integdatasets:
  - dataset: INTEGXT8
    maxlambda: 1e2
  - dataset: INTEGXT3
    maxlambda: 1e2

maxcores: 4
mcseed: 75955222
nnseed: 2080989803

parameters:
  activation_per_layer:
  - tanh
  - tanh
  - linear
  dropout: 0.0
  epochs: 17000
  initializer: glorot_normal
  integrability:
    initial: 10
    multiplier: null
  layer_type: dense
  nodes_per_layer:
  - 25
  - 20
  - 8
  optimizer:
    clipnorm: 6.073e-06
    learning_rate: 0.002621
    optimizer_name: Nadam
  positivity:
    initial: 184.8
    multiplier: null
  stopping_patience: 0.1
  threshold_chi2: 3.5
positivity:
  posdatasets:
  - dataset: POSF2U
    maxlambda: 1e6
  - dataset: POSF2DW
    maxlambda: 1e6
  - dataset: POSF2S
    maxlambda: 1e6
  - dataset: POSFLL
    maxlambda: 1e6
  - dataset: POSDYU
    maxlambda: 1e10
  - dataset: POSDYD
    maxlambda: 1e10
  - dataset: POSDYS
    maxlambda: 1e10
  - dataset: POSF2C
    maxlambda: 1e6
  - dataset: POSXUQ
    maxlambda: 1e6
  - dataset: POSXUB
    maxlambda: 1e6
  - dataset: POSXDQ
    maxlambda: 1e6
  - dataset: POSXDB
    maxlambda: 1e6
  - dataset: POSXSQ
    maxlambda: 1e6
  - dataset: POSXSB
    maxlambda: 1e6
  - dataset: POSXGL
    maxlambda: 1e6

save: weights.h5

theory:
  theoryid: 200
trvlseed: 376191634

scarlehoff · 2024-11-13T10:12:20Z

This is the type of agreement found between old and new branch for a DIS only inconsistent closure test with lambda = 0
https://vp.nnpdf.science/sH-C_csbTGCPCdEGFXj_1w==

I also used parallel_models: true for the new fit. Is this compatible with the type of agreement that is found between standard runned fits and parallel_models run fits? (@scarlehoff)

I would need to see chi2 / distances / etc. By running in parallel the seeding might happen differently at some stage but the initialization (if everything goes well) should be equivalent.

By eye they look too different to me, but if all metrics are very close it could well be a random fluctuation.

RoyStegeman · 2024-12-05T18:54:04Z

Looking at the plots I share @scarlehoff's impression, but indeed a full compare report including chi2s and distances would be needed to say it more conclusively.

From Stefano's comments during the PC I have the impression that the paper is about to be finished, so I think this PR deserves some priority. It would be very good to have this merged before the paper is on arxiv, because otherwise I'm afraid it may never happen.

comane · 2025-02-13T12:06:42Z

@scarlehoff , @RoyStegeman , and @giovannidecrescenzo

I have tested this module by running a multiclosure test for the DIS bulk inconsistency scenario with an inconsistency parameter lambda = 0.
The results (see report below) seem to be in good agreement with what we find with the other branch and also have in the paper draft.

Because of this I think that this branch is ready to be merged

https://vp.nnpdf.science/idXftjIBTp6r2ufbFIhMaA==

scarlehoff

Great! I've left a few comments, mostly related to reducing the reach of the changes inside validphys (I think the more funcionality is shared the better it will be for future maintainability).

Other than that it looks good I think.

validphys2/src/validphys/config.py

validphys2/src/validphys/filters.py

scarlehoff · 2025-02-13T12:22:18Z

validphys2/src/validphys/filters.py

+    inconsistent_data_settings,
+):
+    """
+    Same as _filter_closure_data, but for inconsistent closure tests.


It would be good to add when / what makes the difference with respect to _filter_closure_data.

I think that, since this an internal function (where all the funcionality is) and not an action, it would be greatly benefitial to have the functionality inside _filter_closure_data with a switch (it is probably enough to check whether inconsistent_data_settings is None.

Something like adding:

if inconsistent_data_settings is not None: # Modify the closure data adding inconsistencies. closure_data = [ InconsistentCommonData( setname=cd.setname, ndata=cd.ndata, commondataproc=cd.commondataproc, nkin=cd.nkin, nsys=cd.nsys, commondata_table=cd.commondata_table, systype_table=cd.systype_table, ) for cd in closure_data ]

directly to _filter_closure_data

I don't completely agree on this. the function is internal and used as an explicit node by the config module.

No, the one used as a node is filter_inconsistent_closure_data_by_experiment, I mean _filter_closure_data and _filter_inconsistent_closure_data

Yes sorry meant that one.
What I mean is that I don't understand why you want
filter_inconsistent_closure_data_by_experiment
to be calling the same internal function as
filter_closure_data_by_experiment

they are two separate things and this only adds one extra function (_filter_inconsistent_closure_data )
to filters.
The advantage, in my opinion, is that _filter_closure_data does not become to large as a function

Because if anything changes in how _filter_closure_data behaves now you have to change two functions instead of one.
Or worse, someone down the line will change something in _filter_closure_data and will forget that the inconsistent one has to be changed in the same way and both functions will drift away.

Just to say I agree with this. _filtere_inconsistent_closure_data is an extension of _filter_closure_data that still seems to make use of all of the original functionality so making it two separate functions is not ideal.

Ok, I think there are at least two ways of doing this
I can either leave two separate nodes filter_inconsistent_closure_data_by_experiment and filter_closure_data_by_experiment and let the explicit node deal with them, or reduce even those.

I am actually favourable to having the two separate nodes and a rule in config that depends on whether inconsistent_fakedata has been set True.
This would avoid running inconsistent closure tests in the case in which one has forgotten inconsistent_data_settings in the runcard.

I also had quick comment: as of now we are disregarding the option to generate lvl1 data "inconsistently" in the sense that we do not have anymore the option to vary the generation of lvl 1 data, correct?

Is what you refer here what we used to call type 2 inconsistency?

If so, we never used this and it is not in the paper.

yeah exactly, just to be sure

validphys2/src/validphys/closuretest/inconsistent_closuretest/inconsistent_ct.py

… property

…s it is done for the central values

RoyStegeman

Thanks, I don't have much to add to JCM's comments.

Please add documentation

validphys2/src/validphys/config.py

RoyStegeman · 2025-02-13T13:39:24Z

validphys2/src/validphys/filters.py

+    inconsistent_data_settings,
+):
+    """
+    Same as _filter_closure_data, but for inconsistent closure tests.


Just to say I agree with this. _filtere_inconsistent_closure_data is an extension of _filter_closure_data that still seems to make use of all of the original functionality so making it two separate functions is not ideal.

… tests

comane · 2025-02-14T12:18:41Z

@scarlehoff, @RoyStegeman If you are happy with the current state of this PR, I would suggest merging it.

scarlehoff

Thanks. It might be nice to refer to the paper once it is out in the docstr for context.

Just a last nitpic, did you use pre-commit? I think isort would've picked up some of the issues (you can just touch the files, then git add them and do precommit, otherwise just run isort on all of them -also the test-).

validphys2/src/validphys/filters.py

comane · 2025-02-14T17:11:25Z

Thanks for all the comments, they should all have been addressed

comane added closure tests validphys labels Oct 17, 2024

comane requested review from scarlehoff, RoyStegeman and giovannidecrescenzo October 17, 2024 14:35

comane force-pushed the introduce_inconsistent_fit_data branch 3 times, most recently from 36371a3 to 809dd6d Compare October 20, 2024 12:01

RoyStegeman mentioned this pull request Dec 5, 2024

Introduction of an inconsistency within a Closure Test #1682

Open

giovannidecrescenzo approved these changes Feb 10, 2025

View reviewed changes

giovannidecrescenzo self-requested a review February 10, 2025 14:27

scarlehoff reviewed Feb 13, 2025

View reviewed changes

comane added 12 commits February 13, 2025 12:31

added inconsistent_data_settings parser

83cc1a1

added module with InconsistentCommonData class

11d6517

added explicit node for filtering of inconsistent fakedata

edd9cc6

added _filter_inconsistent_closure_data to filters

21aa899

added test for inconsistent coredata commondata class

40175c2

renamed type err to treatment err

957b945

add log warning for ict and avoid circular import error

c93786b

changed names to treatmnet and names)uncertainties

b895f54

adapted inconsistent cd class to write on systematics table

f49a796

override systematic_errors method with a property and added setter to…

e52b373

… property

reindex systematic errors for inconsistent datasets in the same way a…

348cd3b

…s it is done for the central values

adapted tests

2072d85

comane force-pushed the introduce_inconsistent_fit_data branch from 809dd6d to 2072d85 Compare February 13, 2025 12:33

comane added 2 commits February 13, 2025 12:36

raise error when unknown dataset key is specified

47410ec

added docstring on inconsistent_fakedata

be57cbd

comane added 2 commits February 13, 2025 12:51

updated docstrings in filters module

bfecfdf

use yaml_safe from utils

32f1e9f

RoyStegeman reviewed Feb 13, 2025

View reviewed changes

comane added 3 commits February 13, 2025 14:39

added doc to parse inconsistent_data_settings

4edd616

use _filter_closure_data for both consistent and inconsistent closure…

d9c01c2

… tests

addressed review comments

2e1e650

scarlehoff approved these changes Feb 14, 2025

View reviewed changes

validphys2/src/validphys/filters.py Outdated Show resolved Hide resolved

validphys2/src/validphys/filters.py Outdated Show resolved Hide resolved

validphys2/src/validphys/filters.py Outdated Show resolved Hide resolved

improved readability and use with to export data to file

69dbfa4

comane merged commit 9e82a95 into master Feb 14, 2025
9 checks passed

comane deleted the introduce_inconsistent_fit_data branch February 14, 2025 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce inconsistent fit closure test data #2180

Introduce inconsistent fit closure test data #2180

comane commented Oct 17, 2024 •

edited

Loading

scarlehoff commented Nov 13, 2024

RoyStegeman commented Dec 5, 2024

comane commented Feb 13, 2025

scarlehoff left a comment

scarlehoff Feb 13, 2025

comane Feb 13, 2025

scarlehoff Feb 13, 2025

comane Feb 13, 2025

scarlehoff Feb 13, 2025

RoyStegeman Feb 13, 2025

comane Feb 13, 2025

giovannidecrescenzo Feb 13, 2025

comane Feb 13, 2025

giovannidecrescenzo Feb 13, 2025

RoyStegeman left a comment

RoyStegeman Feb 13, 2025

comane commented Feb 14, 2025

scarlehoff left a comment

comane commented Feb 14, 2025 •

edited

Loading

Introduce inconsistent fit closure test data #2180

Introduce inconsistent fit closure test data #2180

Conversation

comane commented Oct 17, 2024 • edited Loading

New Class and Methods:

Configuration Updates:

Filtering Methods:

Testing:

Benchmarking of Code

Test 0.0

Multiclosure Test

scarlehoff commented Nov 13, 2024

RoyStegeman commented Dec 5, 2024

comane commented Feb 13, 2025

scarlehoff left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RoyStegeman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

comane commented Feb 14, 2025

scarlehoff left a comment

Choose a reason for hiding this comment

comane commented Feb 14, 2025 • edited Loading

comane commented Oct 17, 2024 •

edited

Loading

comane commented Feb 14, 2025 •

edited

Loading