Closed alecandido closed 8 months ago
Is Clone
automatically propagated to Python by PyO3?
Not sure about what do you mean, but consider that:
Clone
myself
https://github.com/NNPDF/pineappl/blob/d819050890c47426bc7fb5bb3f7ab69ced1c3ac8/pineappl_py/src/grid.rs#L103C1-L103C1Clone
any longer, because they are just Python objectsAre we missing something else then a unit test here? or why is this draft mode?
A test would be nice, but I'm also OK merging it if someone tests it and reports success.
I was waiting to write the test, that's why is still draft :)
@cschwan I added a stub of the test for Grid.merge
, but it's currently broken.
I didn't dig too much myself, but if you could take a look, maybe you could guess immediately what I'm doing wrong.
Moreover, the test is pretty basic, since I'm just counting the number of bins (assuming that merge
is possibly extending those). Do you have ideas for a better one?
I think you have non-overlapping bin limits, try changing the limits of the second grid.
I tried to change, but it is still not working: I expect to add new bins into the merged grid, but I always find the same ones.
Unfortunately, there is little documentation about Grid::merge
:
https://github.com/NNPDF/pineappl/blob/c756b4ca839804212c8432d9f2a5491e9d6e6202/pineappl/src/grid.rs#L866-L875
So, if this becomes very relevant, I will dig into the code. But remaining at a superficial level (without exploring PineAPPL itself), I'm not able to tell if there is a bug in the (minimal) Python layer, or I'm misunderstanding Grid::merge
usage.
That seems to be the first problem:
tests/test_grid.py:23: in fake_grid
g = pineappl.grid.Grid.create(lumis, orders, bin_limits, subgrid_params)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
cls = <class 'pineappl.grid.Grid'>
lumi = [<builtins.PyLumiEntry object at 0x7ff7ff8f62d0>]
orders = [<builtins.PyOrder object at 0x7ff7ff8f63f0>]
bin_limits = array(None, dtype=object)
subgrid_params = <pineappl.subgrid.SubgridParams object at 0x7ff7ff42b6d0>
@classmethod
def create(cls, lumi, orders, bin_limits, subgrid_params):
"""Create a grid object from its ingredients.
Parameters
---------
lumi : list(LumiEntry)
List of active luminosities
orders: list(Order)
List of available orders
bin_limits: sequence(float)
Bin limits
subgrid_params : SubgridParams
subgrid parameters
"""
lumi = [lentry.raw for lentry in lumi]
orders = [o.raw for o in orders]
> return cls(PyGrid(lumi, orders, np.array(bin_limits), subgrid_params.raw))
E TypeError: argument 'bin_limits': type mismatch:
E from=object, to=float64
pineappl/grid.py:84: TypeError
If I read this error message properly, than the parameter bin_limits
of the Grid
constructor is an array of objects:
bin_limits = array(None, dtype=object)
Are you missing the .0
when creating the array?
Thanks for the help, but unfortunately you're looking at the wrong example (not sure why that one, i.e. test_fill_all
, is now failing, if not failing on master
).
The one I'm now developing is failing here: https://github.com/NNPDF/pineappl/actions/runs/6625861875/job/17997650555?pr=247#step:4:984 and the error is the following:
> assert g.bins() == 4
E assert 2 == 4
E + where 2 = <built-in method bins of builtins.PyGrid object at 0x5574f0dfda70>()
E + where <built-in method bins of builtins.PyGrid object at 0x5574f0dfda70> = <pineappl.grid.Grid object at 0x7ff7ff463b90>.bins
Actually, I even know where is the problem you were looking: it is now taking the default value, i.e. None
, I will fix it immediately.
Actually, the two problems were somehow connected: fixing ones I've been able to fix the other.
However, the bins should be either consecutive (and so non-overlapping) or coinciding (inferred by trials), but this behavior is documented only by the errors I got.
In any case: if you have no better test in mind, I'm ready to merge.
Since
Grid
is nowClone
, we can deprecatePyGrid::merge_from_file
from the Python API and replace with justPyGrid::merge
.