Support different muf2/x1/x2 grid values

[cschwan]

This branch relaxes the requirements on FkTables, which before required the same x1 and x2 grid x values, if both of the initial states are hadronic. The method Grid::axes has been rewritten such that it allows for this case, and only checks if across the luminosities, but for the same order and bin the x1 grids agree with each other and separately that the x2 grids agree with each other. In Grid::convolute_eko this requires a translation of source-x values of the grid to the source-x values of the operator. This generalizes the case in which they were inversely sorted to each other.

This PR closes #92.

CC @AleCandido @felixhekhorn we need to test this, but I'd like to avoid creating a new Python API release for this branch. Specifically we need to check that 1) it doesn't introduces a regression, meaning that grids where the x1 and x2 values are all the same are still properly evolved, 2) that grids where x1 and x2 values are different are now working as well.

[codecov[bot]]

Codecov Report

Merging #93 (85ccc72) into master (64d5cec) will increase coverage by 0.26%. The diff coverage is 89.79%.

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   85.74%   86.00%   +0.26%     
==========================================
  Files          14       14              
  Lines        2076     2080       +4     
==========================================
+ Hits         1780     1789       +9     
+ Misses        296      291       -5     

Impacted Files	Coverage Δ
pineappl/src/grid.rs	84.68% <89.79%> (+0.99%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 64d5cec...85ccc72. Read the comment docs.

[alecandido]

CC @AleCandido @felixhekhorn we need to test this, but I'd like to avoid creating a new Python API release for this branch. Specifically we need to check that

I don't know what are you thinking about (we can test even without python), but in any case in which you need python you can always compile the API with maturin develop, as we should do in the unit tests as well. And we can do the same even for fktables generation and comparison, no need for a new release :)

[felixhekhorn]

If you wont to bypass Python here, I guess the best you can do would be a identity operator

[cschwan]

[..] you can always compile the API with maturin develop, as we should do in the unit tests as well. And we can do the same even for fktables generation and comparison, no need for a new release :)

That's what I meant!

[felixhekhorn]

Feel free to use my dom scripts ;-) (you will need to generate a new EKO most likely though) otherwise I will continue there on Monday ...

[felixhekhorn]

alright, so axes didn't cause an error (EKO is generated right now ...), but the result for LHCB_Z_13TEV_DIELECTRON looks a bit suspicious:

Q2grid:
- 8315.170000000013
- 8315.179999993466
- 8315.189999999982
[...]
targetgrid:
- 5.294349915558209e-06
- 5.294349915558228e-06
[...]
- 1.9800545836232018e-05
- 1.980054583623205e-05
- 1.9800545836232123e-05
[...]

• the Q2 grid looks a bit hacky - something @AleCandido and I used to do when bypassing some caching
• there are 3(!) x points almost identical - does this mean one of the grids already contains two very close points?

[cschwan]

@felixhekhorn Thanks a lot for testing this!

• the Q2grid looks OK to me, what's the problem that you see here? These points are unnecessarily close to each other, but that's just how APPLgrid 'optimizes' this grid.
• The targetgrid looks like it can be improved using approx_eq instead of a strict equality operator, which is what's being done now.

[felixhekhorn]

* the `Q2grid` looks OK to me, what's the problem that you see here? These points are unnecessarily close to each other, but that's just how APPLgrid 'optimizes' this grid.

no problem, it's just a bit costly - but if APPL works that way, we can do it ...

* The `targetgrid` looks like it can be improved using `approx_eq` instead of a strict equality operator, which is what's being done now.

did you commit already?

Now, running we get:

$ python run.py 
Found grid for LHCB_Z_13TEV_DIELECTRON
┌───────────────┐
│ Computing ... │
└───────────────┘
   /media/FK/fktables/data/appl_grids/200-LHCB_Z_13TEV_DIELECTRON.pineappl.lz4
 + /media/FK/fktables/data/appl_operators/200-LHCB_Z_13TEV_DIELECTRON.yaml
 = /media/FK/fktables/data/appl_fktables/200-LHCB_Z_13TEV_DIELECTRON.pineappl
thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `829`,
 right: `50`', /media/FK/pineappl/pineappl/src/grid.rs:1332:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Traceback (most recent call last):
  File "/media/FK/fktables/run.py", line 203, in <module>
    build_fktables(theory_id, grids, operators_base_path, fktable_base_path, pdf)
  File "/media/FK/fktables/run.py", line 195, in build_fktables
    pineko.convolute(grid, operator_path, fktable_path, pdf)
  File "/media/FK/pineko/src/pineko/__init__.py", line 118, in convolute
    fktable = pineappl_grid.convolute_eko(operators, "evol", order_mask=order_mask_qcd)
  File "/media/FK/env/lib/python3.9/site-packages/pineappl/grid.py", line 247, in convolute_eko
    self.raw.convolute_eko(
pyo3_runtime.PanicException: assertion failed: `(left == right)`
  left: `829`,
 right: `50`

[cschwan]

Commit 9c05e7e6d7c37fe3ad3d57b96533d747d95fd479 implements the changes mentioned above. Could you please check and see if works as you expect it? We might have to tune the ulps parameters a bit, but for the example you reported it should work.

I'll take care of the panic in the meantime.

[cschwan]

Commit c1d0f8cebac52a2fb590a5055af9cde403e1633f should fix the panic.

[felixhekhorn]

Commit 9c05e7e implements the changes mentioned above. Could you please check and see if works as you expect it? We might have to tune the ulps parameters a bit, but for the example you reported it should work.

the size got reduzed, but still we have very close points:

- 2.9711962541436296e-05
- 2.9711962541436452e-05

keep in mind that we typically estimate the interpolation to be at latest a relative error of 1e-6 - so, I think, if we assume 10 common digits that should be more then enough ...

[cschwan]

By the way, the above panic message says that the dataset needs 829 (fewer after commit https://github.com/N3PDF/pineappl/commit/9c05e7e6d7c37fe3ad3d57b96533d747d95fd479) x-grid points. That's just ridiculous... But the APPLgrid optimization works completely differently, as follows: "here are 40 Q^2 and 50 x-grid points: use all of them, and optimize the interpolation error by adjusting their values". Obviously this uses many more grid points than we do right now.

[cschwan]

@felixhekhorn For the time being I've adjust ulps from 32 to 64. In the future we might want to support a relative error like you suggested.

[cschwan]

@felixhekhorn There will be many more panics in convolute_eko, I'm afraid, we should probably meet and fix it together...

[felixhekhorn]

@felixhekhorn There will be many more panic in convolute_eko, I'm afraid, we should probably meet and fix it together...

yes! before 4d133 I still get

$ python run.py                                                                      │remote: Total 5 (delta 3), reused 5 (delta 3), pack-reused 0
Found grid for LHCB_Z_13TEV_DIELECTRON                                                                   │Unpacking objects: 100% (5/5), 583 bytes | 97.00 KiB/s, done.
┌───────────────┐                                                                                        │From github.com:N3PDF/pineappl
│ Computing ... │                                                                                        │   9c05e7e..c1d0f8c  diff-x-grids -> origin/diff-x-grids
└───────────────┘                                                                                        │Updating 9c05e7e..c1d0f8c
   /media/FK/fktables/data/appl_grids/200-LHCB_Z_13TEV_DIELECTRON.pineappl.lz4                           │Fast-forward
 + /media/FK/fktables/data/appl_operators/200-LHCB_Z_13TEV_DIELECTRON.yaml                               │ pineappl/src/grid.rs | 1 -
 = /media/FK/fktables/data/appl_fktables/200-LHCB_Z_13TEV_DIELECTRON.pineappl                            │ 1 file changed, 1 deletion(-)
thread '<unnamed>' panicked at 'assertion failed: `(left == right)`                                      │[felix@dom pineappl_py]$ maturin develop
  left: `50`,                                                                                            │🍹 Building a mixed python/rust project
 right: `651`', /media/FK/pineappl/pineappl/src/grid.rs:1333:9                                           │🔗 Found pyo3 bindings
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace                            │🐍 Found CPython 3.9 at python
Traceback (most recent call last):                                                                       │Requirement already satisfied: numpy~=1.21.0 in /media/FK/env/lib/python3.9/site-packages (1.21.3)
  File "/media/FK/fktables/run.py", line 203, in <module>                                                │   Compiling pineappl v0.5.0-beta.4 (/media/FK/pineappl/pineappl)
    build_fktables(theory_id, grids, operators_base_path, fktable_base_path, pdf)                        │   Compiling pineappl_py v0.5.0-beta.4 (/media/FK/pineappl/pineappl_py)
  File "/media/FK/fktables/run.py", line 195, in build_fktables                                          │    Finished dev [unoptimized + debuginfo] target(s) in 3.13s
    pineko.convolute(grid, operator_path, fktable_path, pdf)                                             │[felix@dom pineappl_py]$ 
  File "/media/FK/pineko/src/pineko/__init__.py", line 118, in convolute                                 │
    fktable = pineappl_grid.convolute_eko(operators, "evol", order_mask=order_mask_qcd)                  │
  File "/media/FK/env/lib/python3.9/site-packages/pineappl/grid.py", line 247, in convolute_eko          │
    self.raw.convolute_eko(                                                                              │
pyo3_runtime.PanicException: assertion failed: `(left == right)`                                         │
  left: `50`,                                                                                            │
 right: `651`              

[alecandido]

I removed codecov annotations from github (as I did for our other repos): we're continuously ignoring them, and they are cluttering the diffs.

For anything else, codecov is still working as before.

P.S.: this only applies for new commits, old ones are cluttered forever... (you can hide them by pressing a, but things that were unfolded will stay as they are)

[cschwan]

@felixhekhorn you can give it one more try.

[felixhekhorn]

Now, we entered - however, I killed it afterwards looking at the timing ...

$ python run.py 
Found grid for LHCB_Z_13TEV_DIELECTRON
┌───────────────┐
│ Computing ... │
└───────────────┘
   /media/FK/fktables/data/appl_grids/200-LHCB_Z_13TEV_DIELECTRON.pineappl.lz4
 + /media/FK/fktables/data/appl_operators/200-LHCB_Z_13TEV_DIELECTRON.yaml
 = /media/FK/fktables/data/appl_fktables/200-LHCB_Z_13TEV_DIELECTRON.pineappl
[00:02:40] ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░      21/39984   - ETA: 3d 11:50:51 

shall I keep it running?

[alecandido]

The progress bar is completely weird...

[cschwan]

It's very likely bugged, and possibly faster. Keep it running and let's see what happens.

[cschwan]

We need to discuss the following point: whenever the source and target x grids differ, we need to think carefully about what the target x grid should be. Setting the target grid to the source grid (status quo) is certainly not a good idea.

[alecandido]

We need to discuss the following point: whenever the source and target x grids differ, we need to think carefully about what the target x grid should be. Setting the target grid to the source grid (status quo) is certainly not a good idea.

We have to think carefully and discuss, I agree, however from the point of view of EKO, it would be better if the smaller one is a subset of the other (you should not suffer too much from re-interpolation).

Moreover, consider that EKO allows for actually 3 different grids: the grid that is used internally for the calculation might be different from both of the other (still it is both input and output at an early stage, so the constraint of being a subset still applies, and it is recommended to be the internal one to be a superset of both).

[cschwan]

@AleCandido @felixhekhorn I'd like to merge this. Is anything missing?

[felixhekhorn]

I think should be fine!