Shapley analysis#2486
Conversation
setup.py: merge V and V15 into a compound player. Add _build_full_covmat / build_full_covmat_for_observables to build an inter-dataset covariance matrix from shared named systematics (HC_*, NUCLEAR*, lumi); setup_observables now returns the full L_inv and concatenated central data vector. analyzer.py: expand compound players in the sign matrix (shared sign column and amplitude per constituent); optional full-covmat chi2 path; new NNPDFShapleyAnalyzerVecX for (flavor x x-region) player games. perturbation.py: apply_multi_gaussian_perturbation for superposed per-player bumps with single positivity enforcement.
vp_shapley.py: flavor basis now always uses 8 players with compound c+ (indices [2,10] sharing one sign column) instead of the previous 9-player scheme that treated c and cbar independently. Evolution basis gets the compound V/V15 player from setup.py automatically. use_full_covmat is read from the runcard and passed to both the main analyzer and any stabilization-rerun analyzer (which rebuilds the covmat for the surviving dataset subset). .gitignore: exclude slurm/logs/.
- nnpdf4.0_calibrated_dense: 82 datasets, 50 log-spaced x in [1e-4, 0.9], use_full_covmat: true, enforce_sumrules: true (production runcard) - nnpdf4.0_ablation: 62 datasets, no covmat, no sum rules (ablation study) - New _ev variants: same games in evolution basis - DIS-FT, HERA_dense, DY_rap, JET_ev, tt_ev: process-specific subsets - k_fold: 4-fold cross-validation splits - Remove NU_fixed, DY_other, nnpdf4.0_calibrated_evol (superseded)
|
|
||
| [tool.poetry.dependencies] | ||
| # Generic dependencies (i.e., validphys) | ||
| python = ">=3.9" | ||
| shapley-values = {git = "https://github.com/rbonnetguerrini/shapley-values.git", branch = "main"} |
There was a problem hiding this comment.
I would suggest @rbonnetguerrini to make a release of his code and upload to PyPI, so we avoid installing from git.
There was a problem hiding this comment.
This will be necessary before merging to master yes, but we can wait.
scarrazza
left a comment
scarrazza
left a comment
There was a problem hiding this comment.
@rbonnetguerrini after discussing with @scarlehoff, we think we should add a section in the sphinx documentation. For example, you could move the execution tutorial from the scripts readme directly there (see doc folder) and include the example in the jupyter notebook.
|
Hi @rbonnetguerrini I think if you rebase on top of the current master (rebase would be much preferred and since this is a separate feature it should be painless) most of the test will pass. |
scarlehoff
left a comment
scarlehoff
left a comment
There was a problem hiding this comment.
Just a few first comments.
From a quick look I think many functions should be using internal valiphys function instead of mirrors of it to ensure things are taken in exactly the same way. I am mainly worried about the covariance matrices but it really applies to everything.
But I think most of the issue is setup_observables which should be modified to be as small as possible, reusing as much internal validphys stuff as possible, I would do the following:
- Use the API instead of Loader, and ask the API for the different pieces that you need (covmat, fktables, etc). Then from those outputs you construct your own output (instead of having to compute the covmat yourself it would be
API.covmat_inverse_whatever_something_else(**config) - With that you will know which input do you actually need
- Remove all functions that will become unused at that point
Then, later instead of calling it in the script with main() it can be called as a subclass of ValidphysApp.run() and the API part can also be removed.
|
|
||
| [tool.poetry.dependencies] | ||
| # Generic dependencies (i.e., validphys) | ||
| python = ">=3.9" | ||
| shapley-values = {git = "https://github.com/rbonnetguerrini/shapley-values.git", branch = "main"} |
There was a problem hiding this comment.
This will be necessary before merging to master yes, but we can wait.
| @@ -0,0 +1,292 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
It worries me that such a long slurm script is necessary to run this?
In any case, I would prefer just documentation, I don't think uploading slurm scripts to the repo is necessary (since they would only work for the system you were running this on, most clusters are configured differently).
| @@ -0,0 +1,95 @@ | |||
| # | |||
There was a problem hiding this comment.
I think the runcards should go to the paper repository, which is where we have most of the "reproduce X paper" runcards.
In this repository we should have just one example runcard at most (in a folder called example or something like that). Or even none, if the whole example can be written in the documentation.
| import pandas as pd | ||
| from scipy.linalg import cholesky, solve_triangular | ||
|
|
||
| from validphys.loader import Loader |
There was a problem hiding this comment.
Instead of Loader, if you use FallbackLoader, when you load something that is not available locally (but it is available in the NNPDF server) it will be automatically downloaded.
|
|
||
| special_sys = pd.concat(special_corrs, axis=0, sort=False).fillna(0) | ||
| diag = la.block_diag(*block_diags) | ||
| return diag + special_sys.to_numpy() @ special_sys.to_numpy().T |
There was a problem hiding this comment.
Why do you need functions like this one?
| def setup_observables( | ||
| pdf_name: str, | ||
| datasets: list, | ||
| theory_id: int = 708, |
There was a problem hiding this comment.
| theory_id: int = 708, | |
| theory_id: int = 40000000, |
4 participants
Includes Shapley analysis tools to the NNPDF framework.