search

NNPDF / pineappl

PineAPPL is not an extension of APPLgrid
https://nnpdf.github.io/pineappl/
GNU General Public License v3.0
13 stars 3 forks source link

Add functionality to remove bins from grids #52

Closed cschwan closed 2 years ago

cschwan commented 3 years ago

We need a method in Grid and a subcommand in the CLI to remove specific bins from a PineAPPL grid.

@felixhekhorn @AleCandido

cschwan commented 2 years ago

Commit 46f3e5f96bed627d5745536fb6074cbdc95b8ec7 adds a method Grid::delete_bins which does what is needed at a very basic level. We still need:

alecandido commented 2 years ago

Python bindings provided in cd7bbaf298326fc5edcdcb2b75868784816dfe5e

It builds, but I had some difficulties to test it. I tried in the interpreter, but it seems not to change the grid...

cschwan commented 2 years ago

I'm preparing a test to check this and there seems to be a problem indeed; Grid::delete_bins seems to have no effect.

cschwan commented 2 years ago

Commit fccd89065b1df9693ef20e820d13ded9db5f240f fixes the problem, and commit 191f62ccad633013357e969cfad4406963dfc895 makes sure it stays fixed. However, the current implementation has the limitation that only bins from the left and/or from the right can be removed. Is that a problem for our purposes?

cschwan commented 2 years ago

Commit 5e629bff0315c69a0fd7693654cc57dfcdab49d7 fixes yet another bug and commit aeddd4fb907ec0fbb91cc01972e539f8bcf67c41 adds the CLI.

alecandido commented 2 years ago

However, the current implementation has the limitation that only bins from the left and/or from the right can be removed. Is that a problem for our purposes?

I'm not sure to have understood what the limitation consists of...

Can you make an example of something I can't do?

cschwan commented 2 years ago

Say you have 10 bins you can't delete 3 and 4, for instance, because they are neither at the left (first bin would be 0) nor are they at the right (last bin would be 9). You could, however, delete bin 3 and 4 if you'd also delete 0, 1 and 2 at the same time, or alternatively if you'd delete 5, 6, 7, 8 and 9.

alecandido commented 2 years ago

Ok, now I understand, and it might be a limitation in general, but (if I remember correctly) the filters in apfelcomb.db should be all at the beginning.

In any case, we'll raise a dedicated issue if and only if we find something for which is needed. For the time being it is perfect as it is.

(I was confused by the words left and right, since each bin has a left end and a right end, but now I perfectly understand)

cschwan commented 2 years ago

I was bit imprecise, so thanks for asking for a clarification! I agree with your proposal to open a separate Issue when this becomes a problem, and we'll know that as soon as the todo!() macro panics.

felixhekhorn commented 2 years ago

Unfortunately we will hit this case ... since I had already a very preliminary look at the cutting, I realized this already (but thought back then, that in fact you did NOT have this limitation): the case is DYE866P

cschwan commented 2 years ago

OK, no problem, I'll take care of it.

alecandido commented 2 years ago

Oh damn, you're right.

There are also a couple of CMS grids.

cschwan commented 2 years ago

Removing any bins should work since commit 50c12c41036d2a54edd8b4cec0e9db3e9da1a4ec.