Extend existing grid

[alecandido]

Essentially split part of the Grid::merge function, for direct usage.

[alecandido]

@cschwan this is just the raw proposal, just to give you an idea of what I had in mind.

I'm not fully satisfied of the result, mainly because of bins treatment: the function is extending bins as well, because I didn't want to touch Grid::increase_shape, and this was the only chance to use this function in Grid::merge.
However, the bins shape is adjusted, but the actual bin_info is untouched, and thus inconsistent.
I will try in a further commit to have for bins a similar treatment to the orders and entries.

(There are a couple of other things that I don't like: the further .clone() of orders and entries, in principle unneeded for Grid::merge, and the duplicated iterations for orders and entries, that was like that even before this PR, but I'd like to deduplicate...).

[cschwan]

This looks OK, except the bins parameter, which is lacking bin limits. Without these the internal bin limits don't match the bin-dimension of the subgrids array and things will go awry.

[alecandido]

This looks OK, except the bins parameter, which is lacking bin limits. Without these the internal bin limits don't match the bin-dimension of the subgrids array and things will go awry.

I had a try with bins as well, but they are definitely more complex, so notice the following differences:

• I'm not processing bins in Grid::extend: whatever you're passing, it will directly finish in the grid (while orders and entries are deduplicated)
• since they are not modified, I'm not returning them in the return value

Moreover, check that the shape increase for bins is correctly computed. Also tell me if you find the parameters names simple, but coherent (so for bins I'm really in doubt between bins and bin_limits).

[alecandido]

QED: I broke something...

[cschwan]

QED: I broke something...

Did you post that on the wrong page? If not, I don't understand.

[alecandido]

Did you post that on the wrong page? If not, I don't understand.

You got it wrong, maybe it would have been clearer spelled Q.E.D. (i.e. quod erat demonstrandum). In Italian the initialism is different, but in English should be customary to use the Latin version.

[cschwan]

You got it wrong, maybe it would have been clearer spelled Q.E.D. (i.e. quod erat demonstrandum). In Italian the initialism is different, but in English should be customary to use the Latin version.

I see and that I understand, but I was thinking of quantum electrodynamics 😄.

[alecandido]

I see and that I understand, but I was thinking of quantum electrodynamics smile.

I know xD

[alecandido]

It seems like the reason why I failed is that I was not familiar enough with BinLimits.

In any case, the code is terrible, but before refactoring I'd like:

• to pass all tests in the CI (locally PineAPPL was passing, but I don't have the converter, e.g.)
• and to get your (@cschwan) opinion about the idea

[cschwan]

I'm not sure whether you can easily extend in the bin dimension. For 1-dimensional distributions you may want to extend the bins not only to the right, but maybe also to the left. In the general case you also have to make sure to take care of the BinRemapper, if one is there. If one is there, it's likely that the bin dimensions are higher-dimensional and that means you also need to give those bin limits. However if you take care of all of that you've basically written what Grid::merge mostly does.

[alecandido]

For the time being, the current function would be enough for our goals, so I would not go much beyond the current state (I'm still a bit unsatisfied about bins treatment, but not enough to change anything).

If you agree to that, @cschwan, I would:

• write a test for Grid::extend
• add Python bindings to the function
• add a more Pythonic wrapper, with defaults to not extend any dimension (such that you can decide to extend a single one, w/o specifying the others)