Make fastNLO converter part of the CLI

[cschwan]

This implements the suggestion by @AleCandido discussed here: https://github.com/N3PDF/pineappl/discussions/115, following my outline given here: https://github.com/N3PDF/pineappl/discussions/115#discussioncomment-2203806. I tried to keep the changes minimal, and the biggest file is basically copied from the examples directory. You can view the difference here:

--- ../examples/fnlo2pine/fnlo2pine.cpp 2022-01-01 22:37:33.214272784 +0100
+++ src/import/fastnlo.cpp      2022-02-19 11:46:59.233366022 +0100
@@ -1,3 +1,5 @@
+#include "pineappl_cli/src/import/fastnlo.hpp"
+
 #include <cmath>
 #include <cstdlib>
 #include <numeric>
@@ -503,16 +505,8 @@
     return pgrid;
 }

-int main(int argc, char* argv[])
+int this_would_be_main(char const* in, char const* out)
 {
-    if (argc != 3)
-    {
-        return EXIT_FAILURE;
-    }
-
-    std::string in(argv[1]);
-    std::string out(argv[2]);
-
     // TODO: read this from an argument
     uint32_t alpha = 0;

@@ -650,11 +644,13 @@
         }
     }

-    pineappl_grid_write(grids.at(0), out.c_str());
+    pineappl_grid_write(grids.at(0), out);
     pineappl_grid_delete(grids.at(0));

     if (different)
     {
         return 1;
     }
+
+    return 0;
 }

As you can see, I only had to rename the main function, and the argument parsing is now done by clap inside the Rust CLI.

The new subcommand works quite well, but it has the same dependencies as the C++ converter: fastNLO and the PineAPPL C API; the latter is a bit odd given that we use the C and the Rust API at the same time, and this must be improved in the future.

One odd point is that build.rs links fastnlotoolkit statically (with z as a dependency), which I was forced to since linking dynamically it didn't work. I have to investigate why.

Everything is hidden behind the feature import-fastnlo, so you have to compile the CLI using --features import-fastnlo or --all-features. @AleCandido do you want to rename import-fastnlo to fastnlo to keep it simple?

[alecandido]

I don't have a neat opinion on import-fastnlo vs fastnlo: no need to be short from my point of view, but actually fastnlo is already not ambiguous, so it's fine as well.

About linking, I will have a look as well, maybe someone wrote something somewhere about it (for sure such a generic statement is true, the problem is that the result of this query will be long). I know that Rust itself is usually linking all crates statically (so everything but standard library), but everything should be possible somehow. The reference has a chapter about linkage, I'll take my time for reading it, maybe it's enough. There is also a reference to FFI section at the beginning, might be useful as well.

About the code itself this is exactly what I expected, and I'm very glad it is working.

This is perfect for the first shot, but as you said having to use PineAPPL C API is odd, but I believe there is an easy solution for this: we should move most of the code to the Rust side, leaving more atomic functions in C++, rather than performing the full task on that side. The moment things will be ported, we'll be left with a converter application in Rust, and essentially Rust bindings for fastNLO.

I would suggest to move the minimal "fastNLO bindings" on a separate crate, inside this repository, as an external contribution and not directly part of PineAPPL CLI (but a dependency of course). Then we might think about publishing it or not on https://crates.io/, but from my point of view this comes at the end, so it's both far in the future and not needed for our main purpose.

[cschwan]

I don't have a neat opinion on import-fastnlo vs fastnlo: no need to be short from my point of view, but actually fastnlo is already not ambiguous, so it's fine as well.

I changed it to fastnlo in commit 29aa3c568429868dedb1209d3b10616047591006 because I think shorter is better :smile:.

About linking, I will have a look as well, maybe someone wrote something somewhere about it (for sure such a generic statement is true, the problem is that the result of this query will be long). I know that Rust itself is usually linking all crates statically (so everything but standard library), but everything should be possible somehow. The reference has a chapter about linkage, I'll take my time for reading it, maybe it's enough. There is also a reference to FFI section at the beginning, might be useful as well.

If you've got the time you could try and see whether you can reproduce my problem. You'll need to install the C API and fastNLO, and then make the following change:

diff --git a/pineappl_cli/build.rs b/pineappl_cli/build.rs
index 0719736..a0d0061 100644
--- a/pineappl_cli/build.rs
+++ b/pineappl_cli/build.rs
@@ -46,8 +46,7 @@ fn main() {
     println!("cargo:rustc-link-search={}", fnlo_lib_path);

     // TODO: why do I have to link statically?
-    println!("cargo:rustc-link-lib=static=fastnlotoolkit");
-    println!("cargo:rustc-link-lib=z");
+    println!("cargo:rustc-link-lib=fastnlotoolkit");

     // bridging code
     bridge.file("src/import/fastnlo.cpp").compile("cxx-bridge");

That should link fastNLO dynamically, but I get lots of linking failures that complain about missing fastNLO symbols.

About the code itself this is exactly what I expected, and I'm very glad it is working.

This is perfect for the first shot, but as you said having to use PineAPPL C API is odd, but I believe there is an easy solution for this: we should move most of the code to the Rust side, leaving more atomic functions in C++, rather than performing the full task on that side. The moment things will be ported, we'll be left with a converter application in Rust, and essentially Rust bindings for fastNLO.

That's exactly the idea!

I would suggest to move the minimal "fastNLO bindings" on a separate crate, inside this repository, as an external contribution and not directly part of PineAPPL CLI (but a dependency of course). Then we might think about publishing it or not on https://crates.io/, but from my point of view this comes at the end, so it's both far in the future and not needed for our main purpose.

That may be a good idea, basically in the same spirit as my LHAPDF crate.

[alecandido]

This afternoon I'll try to dynamically link.

Trying what you put takes nothing, but I want at least a bit of time to investigate (it's unlikely that doing the same I'll obtain a different result).

[codecov[bot]]

Codecov Report

Merging #120 (15a91fe) into master (2197d40) will decrease coverage by 0.05%. The diff coverage is 0.00%.

:exclamation: Current head 15a91fe differs from pull request most recent head f236fdc. Consider uploading reports for the commit f236fdc to get more accurate results

@@            Coverage Diff             @@
##           master     NNPDF/runcards#120      +/-   ##
==========================================
- Coverage   89.63%   89.57%   -0.06%     
==========================================
  Files          31       32       +1     
  Lines        3088     3090       +2     
==========================================
  Hits         2768     2768              
- Misses        320      322       +2     

Impacted Files	Coverage Δ
pineappl_cli/src/import.rs	0.00% <0.00%> (ø)
pineappl_cli/src/main.rs	100.00% <ø> (ø)

---

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 2197d40...f236fdc. Read the comment docs.

[alecandido]

I'm first reading references, starting from Linkage.

Being about pure Rust, it's not very useful (it's useful for me to properly understand the rest). There is a caveat that is worth to mention:

1. If a static library is being produced, all upstream dependencies are required to be available in rlib formats. This requirement stems from the reason that a dynamic library cannot be converted into a static format. Note that it is impossible to link in native dynamic dependencies to a static library, and in this case warnings will be printed about all unlinked native dynamic dependencies.

In any case, this should not be our case, because it is building an executable. Relevant points for executables are 3. and 4., but 3. is really about pure Rust dependencies, and the only two things relevant are:

A major goal of the compiler is to ensure that a library never appears more than once in any artifact. For example, if dynamic libraries B and C were each statically linked to library A, then a crate could not link to B and C together because there would be two copies of A.

For most situations, having all libraries available as a dylib is recommended if dynamically linking. For other situations, the compiler will emit a warning if it is unable to determine which formats to link each library with.

They are both about possible failures and warnings, but mostly about Rust dependencies. However, I need to read the error messages first.

[alecandido]

Just to make some experience, I tried once all the possible installations:

1. I first tried to build without installing fastNLO: it failed (of course), but with a sensible error message:

   thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', pineappl_cli/build.rs:30:14

   (sensible only reading pineappl_cli/build.rs:30:14, but I could understand) I know it's intuitive, but just in case, I would replace that unwrap with an expect("fastNLO library not found, install it please") or something like

2. then I installed fastNLO and retried: it worked, but with tons of warnings about:

   • unused parameters in header files of fastNLO (a bit stupid, they are headers after all)
   • unused functions in pineappl_cli/src/import.rs (I believe this is work in progress, the moment it will be a proper library it will stop to complain)

   I wonder if it's possible to suppress the first kind of warnings immediately, they are not our fault nor our concern (I'd prefer to have only useful warnings, ones I can act on)

3. then I compiled with your patch above (i.e. attempting to link dynamically) and it gave me a gigantic error; essentially, it is a gigantic call to the cc compiler with a lot of files, plus a reasonably small error message:

   = help: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified                                                                                                                                                                                                
   = note: use the `-l` flag to specify native libraries to link                                                                                                                                                                                                                                                                 
   = note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)                                                                                                                            

   it looks like we're not telling the linker where to look for the dylib (i.e. the .so file), since the build is attempting to make an executable it's reasonable that it has also to link. It seems to me that fnlo-tk-config --ldflags is not called anywhere, the result would be

   -L/home/alessandro/.local/lib -lfastnlotoolkit

   for my user, as it's intuitive. I still don't if we should pass them here: https://github.com/N3PDF/pineappl/blob/43298dc3338de29015d02852ea2ce92e835571f7/pineappl_cli/build.rs#L5 or somewhere else, but I'd expect them to be useful...

[alecandido]

I was joking: I read about cargo:rustc-flags=FLAGS and I thought it was something different, but it seems it's just a shorthand for cargo:rustc-link-lib=[KIND=]NAME and cargo:rustc-link-search=[KIND=]PATH, that you were already passing.

I tried as well, and I'm trying to specify the KIND, but all the combination attempted (dylib or not in link-lib, native or not in link-search) did not work.

The error message is still coming from the linker, and it's always the same, I report just the end:

          [...]
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:658: undefined reference to `fastNLOLHAPDF::~fastNLOLHAPDF()'
          collect2: error: ld returned 1 exit status

  = help: some `extern` functions couldn't be found; some native libraries may need to be installed or have their path specified
  = note: use the `-l` flag to specify native libraries to link
  = note: use the `cargo:rustc-link-lib` directive to specify the native libraries to link with Cargo (see https://doc.rust-lang.org/cargo/reference/build-scripts.html#cargorustc-link-libkindname)

so it seems like the flags are not arriving till there, so I'd keep try in this direction.

No progress to report.

[alecandido]

P.S.: the infinite call to the compiler starts with "cc" "-m64", and it also contains "-lfastnlotoolkit" and "-L" "/home/alessandro/.local/lib" (together with all the dozens of pineappl related flags)

So in principle the flags I believe to be needed are there, thus there is something wrong with them. At this point, I believe I need a tiny fastNLO program to run the compilation with dynamic linking manually, and iterate on a cleaner thing. I'll try a few more iterations, but if you have a suggestion for such a C++ program, you'll save me quite some time of fastNLO exploration (unless I find the bug before, which seems unlikely).

[cschwan]

1. I know it's intuitive, but just in case, I would replace that unwrap with an expect("fastNLO library not found, install it please") or something like

Done in commit 15a91feaab521ab474179c4dd15089919e006118.

[alecandido]

This is correctly linking:

#include <fastnlotk/fastNLOLHAPDF.h>

using namespace std;

int main()
{
    auto file = fastNLOLHAPDF("h1.tab", "CT10");

    return 0;
}

with just

g++ -g -Wall -o exp main.cpp $(fnlo-tk-config --ldflags)
# where the last expand to "-L/home/alessandro/.local/lib -lfastnlotoolkit"

Indeed:

❯ ldd exp
        linux-vdso.so.1 (0x00007ffd2c7b1000)
        libfastnlotoolkit.so.0 => /home/alessandro/.local/lib/libfastnlotoolkit.so.0 (0x00007fdb294d5000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fdb292a2000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fdb29288000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fdb29060000)
        libLHAPDF.so => /home/alessandro/.local/lib/libLHAPDF.so (0x00007fdb28f6b000)
        libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fdb28f4f000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fdb28e69000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fdb2965a000)

[cschwan]

Concerning the linking error I'm afraid there's a problem with ordering: https://stackoverflow.com/questions/45135/why-does-the-order-in-which-libraries-are-linked-sometimes-cause-errors-in-gcc. I think for the time being we can ignore the error, and hopefully the situation will improve when the fastNLO bridge will be in a separate crate.

[alecandido]

I see.

The lengthy (but helpful) SO answer taught me something, but now I start to believe that my limited C++ experience is going to kill me, so I'll give up for the time being (we can really investigate anew the moment we have a separate crate).

Nevertheless, there is something mysterious going on in Cargo build, or in cxx. Since the problem is about not finding -lfastnlotoolkit at the correct time, it should be alleviated (if anything), and not enhanced, by linking dynamically. Or, most likely, it should be irrelevant: fastnlotoolkit spelled out its own dependencies when compiled, so they are not a concern, and now it should come in the same place of the static library. Then, why isn't it working?

Another, more important, thing that I learned once more is that Cargo (in pure Rust) is much better than C/C++ builds, and it's really making my life easier...

[cschwan]

I see.

The lengthy (but helpful) SO answer taught me something, but now I start to believe that my limited C++ experience is going to kill me, so I'll give up for the time being (we can really investigate anew the moment we have a separate crate).

Nevertheless, there is something mysterious going on in Cargo build, or in cxx. Since the problem is about not finding -lfastnlotoolkit at the correct time, it should be alleviated (if anything), and not enhanced, by linking dynamically. Or, most likely, it should be irrelevant: fastnlotoolkit spelled out its own dependencies when compiled, so they are not a concern, and now it should come in the same place of the static library. Then, why isn't it working?

I could be wrong and there's an entirely different reason.

Another, more important, thing that I learned once more is that Cargo (in pure Rust) is much better than C/C++ builds, and it's really making my life easier...

Absolutely, although meson more or less solved C++ builds for me. If you run the fastNLO example and finally compile with ninja -v you should see something like:

[2/2] c++  -o fnlo2pine fnlo2pine.p/fnlo2pine.cpp.o -Wl,--as-needed -Wl,--no-undefined -Wl,-O1 -L/home/cschwan/prefix/lib -Wl,--start-group -lfastnlotoolkit /home/cschwan/prefix/lib/libLHAPDF.so /home/cschwan/prefix/lib/libpineappl_capi.so -Wl,--end-group

It uses -Wl,--start-group and Wl,--end-group to enclose its dependencies and apparently the linker then automatically figures out the correct way to link all dependencies.

[alecandido]

Absolutely, although meson more or less solved C++ builds for me.

I never tried enough Meson, since I learned Rust it's hard for me to desire going "back" to C++. But it might happen that sooner or later I'll have to do it, and C++ is still better than Fortran (for general purpose programming, for linear algebra I actually like Fortran), and then I'll follow the advice and go with Meson (autotools are bloated and tiresome, and since my master thesis I've started hating CMake, it's not simple and it's poorly documented).

Actually, some more info about are beloved build: I wanted to compare the size of the statically linked binary (libfastnlotoolkit.a is 52 MB, I don't know how much LTO can do, but it's going for sure to increase pineappl binary: better to measure than to attempt a prediction).

This is the result for the size of the pineappl binary:

• debug profile, no fastnlo (target/debug/pineappl): 56 MB
• debug profile, fastnlo (target/debug/pineappl): 79 MB
• release profile, no fastnlo (target/release/pineappl): 3.7 MB
• release profile, fastnlo: n/a

Actually, I've not been to compile in release mode the statically linked executable. Interestingly, in this case the problem is not linking fastnlotoolkit to pineappl_cli, but instead linking pineappl_capi to fastnlotoolkit. You can try by yourself running on the current status of the repository the following command:

cargo build --release --features=fastnlo

(swapping the two arguments of cargo makes no difference, fortunately)

[cschwan]

This is the result for the size of the pineappl binary:

* debug profile, no fastnlo (`target/debug/pineappl`): 56 MB

* debug profile, fastnlo (`target/debug/pineappl`): 79 MB

* release profile, no fastnlo (`target/release/pineappl`): 3.7 MB

* release profile, fastnlo: n/a

I can confirm these numbers and interestingly the installed pineappl binary without fastNLO is even smaller with 2.1 MB. I wonder how the 1.6 MB get shaved off.

Actually, I've not been to compile in release mode the statically linked executable. Interestingly, in this case the problem is not linking fastnlotoolkit to pineappl_cli, but instead linking pineappl_capi to fastnlotoolkit. You can try by yourself running on the current status of the repository the following command:

cargo build --release --features=fastnlo

(swapping the two arguments of cargo makes no difference, fortunately)

I can confirm this. But that's just another sign that linking is broken, and it isn't necessarily tied to fastNLO.

[alecandido]

I can confirm these numbers and interestingly the installed pineappl binary without fastNLO is even smaller with 2.1 MB. I wonder how the 1.6 MB get shaved off.

I don't know what it is exactly doing, but I can tell you that if you run:

strip pineappl

on the 3.7 MB executable, it becomes 1.9 MB. So I guess something similar...

[cschwan]

It seems that with Rust 1.59.0 I can link against the dynamic library, strange ...

[alecandido]

Really weird...

Have you tried release profile? (either static or dynamic)

[cschwan]

The --release profile seemingly works as well.

[alecandido]

Unfortunately, for me, it looks like the situation is not changed (but I hope it still might, since it worked for you).

I.e.:

• STATIC (current commit, 15a91feaab521ab474179c4dd15089919e006118) vs DYNAMIC:

• println!("cargo:rustc-link-lib=static=fastnlotoolkit");
• println!("cargo:rustc-link-lib=z");
• println!("cargo:rustc-link-lib=fastnlotoolkit");

• DEBUG (no flag) vs RELEASE (--release)

1. STATIC + DEBUG compiling :heavy_check_mark:
2. STATIC + RELEASE failing :x:
3. DYNAMIC + DEBUG failing :x:
4. DYNAMIC + RELEASE failing :x:

I suspect I might be able to solve at least 2., but I still wonder how (and since it's working for you, I hope even 3. and 4.), but it might be more complicated. What happens for 2. is that (after installing pineappl_capi with cargo cinstall --release --prefix $HOME/.local --all-features, that initially I forgot to) build.rs complaints while compiling the bindings for fastnlo, since it's not finding pineappl_capi functions:

  = note: /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/release/build/pineappl_cli-a9661f3a902dffbb/out/libcxx-bridge.a(fastnlo.o): in function `create_lumi(fastNLOCoeffAddBase*, fastNLOPDFLinearCombinations const&, pineappl_lumi*)':                                                              
          fastnlo.cpp:(.text._Z11create_lumiP19fastNLOCoeffAddBaseRK28fastNLOPDFLinearCombinationsP13pineappl_lumi+0x249): undefined reference to `pineappl_lumi_add'                                                                                                                                                           
          /usr/bin/ld: fastnlo.cpp:(.text._Z11create_lumiP19fastNLOCoeffAddBaseRK28fastNLOPDFLinearCombinationsP13pineappl_lumi+0x984): undefined reference to `pineappl_lumi_add'                                                                                                                                              
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/release/build/pineappl_cli-a9661f3a902dffbb/out/libcxx-bridge.a(fastnlo.o): in function `convert_coeff_add_fix(fastNLOCoeffAddFix*, fastNLOPDFLinearCombinations const&, unsigned long, unsigned int)':                                        
          fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0x64): undefined reference to `pineappl_lumi_new'                                                                                                                                                                 
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0x112): undefined reference to `pineappl_keyval_new'                                                                                                                                                 
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0x138): undefined reference to `pineappl_grid_new'                                                                                                                                                   
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0x148): undefined reference to `pineappl_keyval_delete'                                                                                                                                              
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0x150): undefined reference to `pineappl_lumi_delete'                                                                                                                                                
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0x718): undefined reference to `pineappl_subgrid_new2'                                                                                                                                               
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0x976): undefined reference to `pineappl_grid_replace_and_delete'                                                                                                                                    
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0xa1b): undefined reference to `pineappl_subgrid_import_mu2_slice'                                                                                                                                   
          /usr/bin/ld: fastnlo.cpp:(.text._Z21convert_coeff_add_fixP18fastNLOCoeffAddFixRK28fastNLOPDFLinearCombinationsmj+0xcc8): undefined reference to `pineappl_subgrid_delete'                                                                                                                                             
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/release/build/pineappl_cli-a9661f3a902dffbb/out/libcxx-bridge.a(fastnlo.o): in function `convert_coeff_add_flex(fastNLOCoeffAddFlex*, fastNLOPDFLinearCombinations const&, fastNLO::EScaleFunctionalForm, fastNLO::EScaleFunctionalForm, unsign
ed long, unsigned int, int)':                                                                                                                                                                                                                                                                                                   
          fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x78): undefined reference to `pineappl_lumi_new'                                                                                                                           
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x122): undefined reference to `pineappl_keyval_new'                                                                                                           
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x270): undefined reference to `pineappl_keyval_set_string'                                                                                                    
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x2a5): undefined reference to `pineappl_keyval_set_string'                                                                                                    
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x2c8): undefined reference to `pineappl_grid_new'                                                                                                             
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x2d8): undefined reference to `pineappl_keyval_delete'                                                                                                        
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x2e0): undefined reference to `pineappl_lumi_delete'                                                                                                          
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0xb73): undefined reference to `pineappl_subgrid_new2'                                                                                                         
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x11ed): undefined reference to `pineappl_grid_replace_and_delete'                                                                                             
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x1480): undefined reference to `pineappl_subgrid_import_mu2_slice'                                                                                            
          /usr/bin/ld: fastnlo.cpp:(.text._Z22convert_coeff_add_flexP19fastNLOCoeffAddFlexRK28fastNLOPDFLinearCombinationsN7fastNLO20EScaleFunctionalFormES5_mji+0x1974): undefined reference to `pineappl_subgrid_delete'                                                                                                      
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/release/build/pineappl_cli-a9661f3a902dffbb/out/libcxx-bridge.a(fastnlo.o): in function `this_would_be_main(char const*, char const*)':                                                                                                        
          fastnlo.cpp:(.text._Z18this_would_be_mainPKcS0_+0x777): undefined reference to `pineappl_grid_merge_and_delete'
          /usr/bin/ld: fastnlo.cpp:(.text._Z18this_would_be_mainPKcS0_+0x7b2): undefined reference to `pineappl_grid_scale_by_order'
          /usr/bin/ld: fastnlo.cpp:(.text._Z18this_would_be_mainPKcS0_+0x7ca): undefined reference to `pineappl_grid_optimize'
          /usr/bin/ld: fastnlo.cpp:(.text._Z18this_would_be_mainPKcS0_+0x931): undefined reference to `pineappl_grid_set_remapper'
          /usr/bin/ld: fastnlo.cpp:(.text._Z18this_would_be_mainPKcS0_+0xa02): undefined reference to `pineappl_grid_convolute_with_one'
          /usr/bin/ld: fastnlo.cpp:(.text._Z18this_would_be_mainPKcS0_+0xc2d): undefined reference to `pineappl_grid_write'
          /usr/bin/ld: fastnlo.cpp:(.text._Z18this_would_be_mainPKcS0_+0xc35): undefined reference to `pineappl_grid_delete'
          collect2: error: ld returned 1 exit status

but actually the library is there:

  = note: "cc" [...] "-L" "/home/alessandro/.local/lib" [...] "-lpineappl_capi" [...]

since:

❯ pkg-config --libs pineappl_capi
-L/home/alessandro/.local/lib -lpineappl_capi

and

❯ nm -g lib/libpineappl_capi.a | rg pineappl_grid_
0000000000000000 T pineappl_grid_bin_count
0000000000000000 T pineappl_grid_bin_dimensions
0000000000000000 T pineappl_grid_bin_limits_left
0000000000000000 T pineappl_grid_bin_limits_right
0000000000000000 T pineappl_grid_bin_normalizations
0000000000000000 T pineappl_grid_convolute_with_one
0000000000000000 T pineappl_grid_convolute_with_two
0000000000000000 T pineappl_grid_delete
0000000000000000 T pineappl_grid_export_mu2_slice
0000000000000000 T pineappl_grid_fill
0000000000000000 T pineappl_grid_fill_all
0000000000000000 T pineappl_grid_fill_array
0000000000000000 T pineappl_grid_lumi
0000000000000000 T pineappl_grid_merge_and_delete
0000000000000000 T pineappl_grid_new
0000000000000000 T pineappl_grid_nonzero_mu2_slices
0000000000000000 T pineappl_grid_optimize
0000000000000000 T pineappl_grid_order_count
0000000000000000 T pineappl_grid_order_params
0000000000000000 T pineappl_grid_read
0000000000000000 T pineappl_grid_replace_and_delete
0000000000000000 T pineappl_grid_scale
0000000000000000 T pineappl_grid_scale_by_order
0000000000000000 T pineappl_grid_set_key_value
0000000000000000 T pineappl_grid_set_remapper
0000000000000000 T pineappl_grid_write

(and everything else is there as well, here I grepped for pineappl_grid_ to reduce bit the output)

So I don't know what I'm doing wrong

[alecandido]

For dynamic linking, the situation is the opposite: it doesn't find fastnlo

  = note: /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/debug/build/pineappl_cli-00b510835278c2b6/out/libcxx-bridge.a(import.rs.o): in function `fastNLOCoeffAddFix::GetScaleFactor(int) const':
          /home/alessandro/.local/include/fastnlotk/fastNLOCoeffAddFix.h:46: undefined reference to `speaker::operator[](std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/debug/build/pineappl_cli-00b510835278c2b6/out/libcxx-bridge.a(fastnlo.o): in function `create_lumi(fastNLOCoeffAddBase*, fastNLOPDFLinearCombinations const&, pineappl_lumi*)':
          /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:96: undefined reference to `fastNLOPDFLinearCombinations::CalcPDFLinearCombination(fastNLOCoeffAddBase const*, std::vector<double, std::allocator<double> > const&, std::vector<double, std::allocator<double> > const&, bool) 
const'                                                                                                                                                          
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/debug/build/pineappl_cli-00b510835278c2b6/out/libcxx-bridge.a(fastnlo.o): in function `convert_coeff_add_fix(fastNLOCoeffAddFix*, fastNLOPDFLinearCombinations const&, unsigned long, unsigned int)':
          /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:160: undefined reference to `fastNLOCoeffAddFix::GetTotalScalevars() const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:161: undefined reference to `fastNLOCoeffAddFix::GetTotalScalenodes() const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:218: undefined reference to `fastNLOCoeffAddBase::GetXIndex(int, int, int) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:216: undefined reference to `fastNLOCoeffAddBase::GetNxmax(int) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/target/debug/build/pineappl_cli-00b510835278c2b6/out/libcxx-bridge.a(fastnlo.o): in function `this_would_be_main(char const*, char const*)':
          /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:518: undefined reference to `fastNLOLHAPDF::fastNLOLHAPDF(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int
)'                                                                                                                                                              
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:524: undefined reference to `fastNLOReader::ContrId(fastNLO::ESMCalculation, fastNLO::ESMOrder) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:525: undefined reference to `fastNLOReader::ContrId(fastNLO::ESMCalculation, fastNLO::ESMOrder) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:526: undefined reference to `fastNLOReader::ContrId(fastNLO::ESMCalculation, fastNLO::ESMOrder) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:552: undefined reference to `fastNLOTable::GetCoeffTable(int) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:555: undefined reference to `typeinfo for fastNLOCoeffAddFix'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:555: undefined reference to `typeinfo for fastNLOCoeffBase'                  
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:563: undefined reference to `typeinfo for fastNLOCoeffAddFlex'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:563: undefined reference to `typeinfo for fastNLOCoeffBase'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:596: undefined reference to `fastNLOTable::GetObsBinDimBounds(unsigned int, unsigned int) const'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:605: undefined reference to `fastNLOReader::GetCrossSection(bool)'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:658: undefined reference to `fastNLOLHAPDF::~fastNLOLHAPDF()'
          /usr/bin/ld: /media/alessandro/moneybin/Projects/N3PDF/pineappl/pineappl_cli/src/import/fastnlo.cpp:658: undefined reference to `fastNLOLHAPDF::~fastNLOLHAPDF()'

But the library is installed, and flags are present:

  = note: "cc" [...] "-L" "/home/alessandro/.local/lib" [...] "-lfastnlotoolkit" [...]

❯ nm -g --demangle lib/libfastnlotoolkit.so.0.0.0 | rg fastNLOLHAPDF
0000000000121e60 T fastNLOLHAPDF::SetNFlavor(int)
0000000000121b00 T fastNLOLHAPDF::SetAlphasMz(double)
00000000001223c0 T fastNLOLHAPDF::SetLHAPDFMember(int)
0000000000123e60 T fastNLOLHAPDF::GetAsUncertainty(fastNLO::EAsUncertaintyStyle)
0000000000123110 T fastNLOLHAPDF::GetAsUncertainty(fastNLO::EAsUncertaintyStyle, bool)
0000000000121150 T fastNLOLHAPDF::InitEvolveAlphas()
00000000001262c0 T fastNLOLHAPDF::GetPDFUncertainty(fastNLO::EPDFUncertaintyStyle)
0000000000124340 T fastNLOLHAPDF::GetPDFUncertainty(fastNLO::EPDFUncertaintyStyle, bool)
0000000000122d30 T fastNLOLHAPDF::SetLHAPDFFilename(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)
0000000000123eb0 T fastNLOLHAPDF::GetAsUncertaintyVec(fastNLO::EAsUncertaintyStyle)
0000000000126310 T fastNLOLHAPDF::GetPDFUncertaintyVec(fastNLO::EPDFUncertaintyStyle)
0000000000123fb0 T fastNLOLHAPDF::GetPDFUncertaintyLHAPDF(double, bool)
0000000000121d50 T fastNLOLHAPDF::SetMz(double)
0000000000126410 T fastNLOLHAPDF::InitPDF()
0000000000121c30 T fastNLOLHAPDF::SetNLoop(int)
00000000001219e0 T fastNLOLHAPDF::SetQMass(int, double)
0000000000121f80 T fastNLOLHAPDF::fastNLOLHAPDF(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)
0000000000122e90 T fastNLOLHAPDF::fastNLOLHAPDF(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int)
0000000000122150 T fastNLOLHAPDF::fastNLOLHAPDF(fastNLOTable const&)
0000000000122ff0 T fastNLOLHAPDF::fastNLOLHAPDF(fastNLOTable const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int)
0000000000121f80 T fastNLOLHAPDF::fastNLOLHAPDF(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)
0000000000122e90 T fastNLOLHAPDF::fastNLOLHAPDF(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int)
0000000000122150 T fastNLOLHAPDF::fastNLOLHAPDF(fastNLOTable const&)
0000000000122ff0 T fastNLOLHAPDF::fastNLOLHAPDF(fastNLOTable const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, int)
00000000001219c0 T fastNLOLHAPDF::~fastNLOLHAPDF()
0000000000121950 T fastNLOLHAPDF::~fastNLOLHAPDF()
0000000000121950 T fastNLOLHAPDF::~fastNLOLHAPDF()
0000000000122310 T fastNLOLHAPDF::GetNFlavor() const
0000000000122320 T fastNLOLHAPDF::GetAlphasMz() const
00000000001212e0 T fastNLOLHAPDF::EvolveAlphas(double) const
0000000000122390 T fastNLOLHAPDF::GetIPDFMember() const
00000000001223a0 T fastNLOLHAPDF::GetNPDFMembers() const
00000000001223b0 T fastNLOLHAPDF::GetNPDFMaxMember() const
0000000000122bb0 T fastNLOLHAPDF::PrintPDFInformation() const
0000000000122590 T fastNLOLHAPDF::CalcPDFUncertaintyPlus(std::vector<LHAPDF::PDFUncertainty, std::allocator<LHAPDF::PDFUncertainty> > const&) const
00000000001229e0 T fastNLOLHAPDF::CalcPDFUncertaintySymm(std::vector<LHAPDF::PDFUncertainty, std::allocator<LHAPDF::PDFUncertainty> > const&) const
0000000000122480 T fastNLOLHAPDF::CalcPDFUncertaintyMinus(std::vector<LHAPDF::PDFUncertainty, std::allocator<LHAPDF::PDFUncertainty> > const&) const
00000000001228d0 T fastNLOLHAPDF::CalcPDFUncertaintyCentral(std::vector<LHAPDF::PDFUncertainty, std::allocator<LHAPDF::PDFUncertainty> > const&) const
00000000001227c0 T fastNLOLHAPDF::CalcPDFUncertaintyRelPlus(std::vector<LHAPDF::PDFUncertainty, std::allocator<LHAPDF::PDFUncertainty> > const&) const
00000000001226a0 T fastNLOLHAPDF::CalcPDFUncertaintyRelMinus(std::vector<LHAPDF::PDFUncertainty, std::allocator<LHAPDF::PDFUncertainty> > const&) const
0000000000122af0 T fastNLOLHAPDF::GetXFX(double, double) const
00000000001222f0 T fastNLOLHAPDF::GetNLoop() const
0000000000121240 T fastNLOLHAPDF::GetQMass(int) const
000000000016a780 V typeinfo for fastNLOLHAPDF
0000000000140750 V typeinfo name for fastNLOLHAPDF
000000000016a810 V vtable for fastNLOLHAPDF

[cschwan]

@AleCandido did you try with the latest version of Rust? On my computer both 1.59 and 1.61-nightly work.

[alecandido]

I tried only 1.59, and the result is the one reported.

Can you tell me all the steps you expect to be required on a fresh new system, please? From installing pineappl_capi to setting suitable environment variables, like LD_LIBRARY_PATH (you can exclude rust installation, but please include gcc version).

[cschwan]

@AleCandido you only need fastNLO, pineappl_capi isn't needed anymore.

[alecandido]

Ok, with 305f40d I can compile, I can also do release, and I saw that it's dynamic by default:

❯ ldd release/pineappl
        [...]
        libfastnlotoolkit.so.0 => /home/alessandro/.local/lib/libfastnlotoolkit.so.0 (0x00007fbff4c2b000)
        [...]

(even though I'm trying on a different machine, but I do not expect any difference on the former one).

So, now the question is: what is missing to merge? I didn't try the conversion itself, but if it's working (even for a basic prototype) I guess it might be worth merging :)

A separate goal (for another issue/PR) I'd like to achieve, it's to propagate from the CLI to the Python interface, such that I can directly use the API in runcardsrunner for https://github.com/NNPDF/pinefarm/issues/3.

[cschwan]

@AleCandido have a look at the last commit message; basically it doesn't work yet, there's still a bug that makes the two results differ by a few percent.

Making this part of the core Rust API and writing a Python interface seems like a good idea, we can do this in this Issue.

[alecandido]

@AleCandido have a look at the last commit message; basically it doesn't work yet, there's still a bug that makes the two results differ by a few percent.

I don't use it, so I keep forgetting about commits' long description field. I agree that the bug fix should belong to this PR, but the flex (that I don't what it is, but I can guess) maybe might even be postponed.

Making this part of the core Rust API and writing a Python interface seems like a good idea, we can do this in this Issue.

Okay, so:

• I would suggest/agree to move the mod fastnlo to the main pineappl crate (that I guess is what you call the core Rust API)
• from there it will be simple enough for me to write the Python interface

[cschwan]

In commit c43977af17b999358458c169b27fdbf084272b2b I added the changes that we require for https://github.com/NNPDF/fktables/issues/27, and since commit 059491d3d3613ca18318de83744afdcc465a31d5 the conversion should work properly.

@AleCandido could you please test this on dom? I can't access it right now. You need to install fastNLO as explained above and then install the CLI as follows:

cargo install --features=fastnlo --path pineappl_cli

You should then be able to run

find /path/to/applgrid/repo -name '*.tab' -exec pineappl import {} {}.pineappl.lz4 NNPDF40_nnlo_as_01180

to convert the fastNLO tables into PineAPPL grids; they will be placed in the same directory next to them in the applgrids repository. If you run

pineappl obl --bins CMS_2JET_7TEV.tab.pineappl.lz4

you should see 1 in the norm column. You can double-check the convolution, but if the normalization is set to 1 I'm quite sure that it will work. Copy the newly converted grids over the wrong ones and you should be able to restart evolving them!

[alecandido]

@cschwan I would have done, but unfortunately I'm not able to access as well. I even tried with the INFN VPN, to see if it was a problem related to the actual network you're coming from, but nothing.

Maybe dom is just down...

[cschwan]

Commit fc93e17f32d7c8413db0ceb6a5b844f16cc1daca adds support for flexible-scale grids. I've tested it and it works; however, we should test the conversion in the CI. I'm not sure how to best deal with the (large) fastNLO tables; we could store or download/cache them in the CI.

For reference, https://github.com/N3PDF/pineappl/issues/25#issuecomment-906950501 lists publicly available flexible-scale grids, and the following links have more fixed-scale grids:

• http://www.precision.hep.phy.cam.ac.uk/wp-content/results/fastNLOtables/LHC8-ttbar-2dim-CMS.tar.gz
• http://www.precision.hep.phy.cam.ac.uk/wp-content/results/fastNLOtables/2d-ttbar-LHC-13TeV-3_masses.tar.gz
• http://www.precision.hep.phy.cam.ac.uk/wp-content/results/fastNLOtables/LHC13-ttbar-CMS_bin-fastNLO.tar.gz
• http://www.precision.hep.phy.cam.ac.uk/wp-content/results/fastNLOtables/LHC13-ttbar-CMS_bin-fastNLO-172_5.tar.gz
• http://www.precision.hep.phy.cam.ac.uk/wp-content/results/fastNLOtables/fastNLO-ttbar-NNLO-LHC8-173_3-bin1.tar.gz
• http://www.precision.hep.phy.cam.ac.uk/wp-content/results/fastNLOtables/fastNLO-ttbar-NNLO-LHC13-173_3-bin1.tar.gz

[cschwan]

There are more fastNLO tables in the official source tarball, and most of the grids work just fine. However, the following abort with 'could not access coefficient table':

• NJets_nlo-2jet_0101.pineappl.lz4
• NJets_nlo-2jet_0100.pineappl.lz4
• NJets_nlo-2jet_0102.pineappl.lz4

My guess is that instead of having interpolation tables some orders are K-factors.

Furthermore, there's a table 'NJetEvents_norm_1-12.pineappl.lz4' for which the bin results are wrong by a factor 500:

b    PineAPPL     fastNLO     rel. diff
--+------------+------------+-----------
0  1.4479542e-5 7.2397708e-3 4.9900000e2
1  1.2100194e-5 6.0500968e-3 4.9900000e2
2  1.1870701e-5 5.9353507e-3 4.9900000e2
3  1.1986148e-5 5.9930739e-3 4.9900000e2
4  1.2132772e-5 6.0663858e-3 4.9900000e2
5  1.2340099e-5 6.1700495e-3 4.9900000e2
6  1.2480707e-5 6.2403537e-3 4.9900000e2
7  1.2630516e-5 6.3152581e-3 4.9900000e2
8  1.2806140e-5 6.4030702e-3 4.9900000e2
9  6.4738462e-6 6.4738462e-3 9.9900000e2
10 6.6040672e-6 6.6040672e-3 9.9900000e2
11 6.6632940e-6 6.6632940e-3 9.9900000e2

[alecandido]

My guess is that instead of having interpolation tables some orders are K-factors.

Unfortunately there is no actual documentation of fastNLO: there are the papers, but they do not document the code itself. Do you know/believe that there is a dedicated structure for K-factors?

Furthermore, there's a table 'NJetEvents_norm_1-12.pineappl.lz4' for which the bin results are wrong by a factor 500/1000:

This is really a weirdly regular factor, I guess there should be a reasonable explanation. Something like bins' width again?

[cschwan]

Unfortunately there is no actual documentation of fastNLO: there are the papers, but they do not document the code itself. Do you know/believe that there is a dedicated structure for K-factors?

You can run doxygen over the source code. But I'm reading the source code (the headers mostly) itself, as the documentation is often vague or even wrong in small instances.

[cschwan]

This is really a weirdly regular factor, I guess there should be a reasonable explanation. Something like bins' width again?

This is very likely. As this isn't a important right now I've opened a new Issue for it, NNPDF/runcards#126.

I'd like to merge this PR now, as everything that was working is working with the new converter. Do you agree or do you think there's something missing? You requested a Python API for the conversion, but for that I'll probably open a separate Issue or I'll add it into master directly.

[alecandido]

I perfectly agree on decoupling things: the goal of this PR is written in the name, and it's only about the CLI. As you said, it's definitely not worse than before, and now a proper part of the CLI.

Let me try to compile it one last time, and I'll go quickly through the changes to see if everything looks alright. In any case, if I'll find something that is not strictly needed, I'll just open another issue, I'd like to merge as well.

[cschwan]

Thanks, let me know if it works, @AleCandido!