-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generalize the APPLgrid exporter #285
base: master
Are you sure you want to change the base?
Conversation
* Add support for multi-dimensional and non-consecutive bins by assigning them integer bin limits 0..N after informing the user * Add support for arbitrary Q2, x1, x2 grids * Use the `epsilon` option to of `approx_eq!` to not report a false error
Concerning 1): this is a good idea, it's surely better to do something than to error out. There's probably no meaningful alternative to the standard bin limits 2): The problem with general bin limits is that APPLgrid requires a fixed functional form, which means that the bin limits 3): I'd try changing the For points 2) and 3) it would be good to have grids to test that with. |
|
We can't throw an error, because then optimized grids can't be exported. That would be a big loss. There's no way to reconstruct the interpolation order from the grid points alone, you can use the 50 that we always use for different interpolation orders. However I don't think the interpolation order in the exported APPLgrid is important, usually you don't want to fill them after having them exported.
Set it to |
I could also add a |
I don't think this is needed. |
Okay, I set |
What's missing are tests that check the features that you implemented. We need a PineAPPL grid
Those two cases are possibly broken, so we better make sure to test it. |
|
||
let x_grid = grid | ||
.subgrids() | ||
.slice(s![order, bin, ..]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably doesn't work quite as expected. APPLgrid stores different channels in a single appl_igrid
, which has common parameter values NQ2
, NQ2min
, NQ2max
, NQorder
, Nx
, xmin
, xmax
, xorder
. This means that:
- if several channels have different
SubgridParam
/ExtraSubgridParam
values we should error out, unless this is a DIS grid in which we ignore the second dimension. In PineAPPL the second dimension can be eitherx1
orx2
- the
flat_map
should therefore probably be a simplemap
and we must make sure that for all channels the entries are the same - the same applies also for
mu_grid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OPTIMIZE_SUBGRID_TYPE
optimization can change the ends of the x
and mu2
grid points, right? So I would check if the grid points are equal modulo some endpoints, and then use the union as the x
points that will be passed to APPLgrid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would address one point of it, yes. It'll get interesting if you've got a single channel with different minimums/maximums.
.chain(limits.last().map(|vec| vec[0].1)) | ||
.collect(); | ||
let limits = if integer_bin_limits { | ||
(0..=limits.len()).map(|x| x as f64).collect::<Vec<_>>() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here there's very likely a normalization factor missing. If you change the bin sizes you must correct the cross sections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, thanks. Does APPLgrid also divide by the bin widths in the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAICR yes!
While working on this, I realized: Why are the |
Not every |
But the exporter can't handle these anyway, since APPLgrid can't handle them, right? (At least that's how I understood your first comment in this pull request.) So it would only matter (for the exporter) how the The Also, is the |
One could also properly document a potential |
Yes, APPLgrid only understands the grid-node values spaced as
It does support DIS grids, however, where one of the is trivial (zero and usually one x-grid value in the unused x-dimension)
No it's not updated, but you can't rely on this field, because you can merge any kind of subgrid into any Grid. |
I see, this would be a showstopper for using |
Hi,
I implemented the cases not yet implemented in the APPLgrid exporter. This includes:
0..N
. The ordering is the same as the PineAPPL bins, e.g. given bypineappl read --bins
. An info message about the bin limits is printed to the user when a PineAPPL grid with multi-dimensional and non-consecutive bins is exported.Q2
,x1
,x2
gridsepsilon = 1e-12
toapprox_eq!
in lines 240 & 257. Otherwise, this failed for the grid I tested this with, for the two values0.04979197630496172
and0.04979197630496281
(the difference is only in the last 3 digits). Or would you handle this differently?