Merge pull request #380 from NNPDF/various-fixes

Radonirinaunimi · web-flow · commit a87ce7bd4651 · 2026-03-18T22:06:22.000+01:00
Various API fixes
diff --git a/.github/actions/cache-test-data/action.yml b/.github/actions/cache-test-data/action.yml
@@ -10,7 +10,7 @@ runs:
       uses: actions/cache@v4
       with:
         path: test-data
-        key: test-data-v23
+        key: test-data-v24
     - name: Download test data if cache miss
       if: steps.cache.outputs.cache-hit != 'true'
       run: |
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- added an actual implementation of `pineappl_grid_metadata` and
+  `pineappl_grid_set_metadata` in the APIs
+
+### Fixed
+
+- fixed a bug in the Newton's convergence method by raising the maximum number
+  of iteration
+- fixed a bug in the implementation of `pineappl_channels_add` of the Fortran
+  API
+
 ### Changed
 
 - raised MSRV to 1.94.0
diff --git a/examples/cpp/set-subgrids.cpp b/examples/cpp/set-subgrids.cpp
@@ -207,6 +207,16 @@ int main() {
     );
     pineappl_grid_optimize(grid);
 
+    // ---
+    // Metadata testing
+    pineappl_grid_set_metadata(grid, "comment", "This is a toy SIDIS grid");
+    pineappl_grid_set_metadata(grid, "experiment", "ToyExp");
+
+    char* comment = pineappl_grid_metadata(grid, "comment");
+    char* experiment = pineappl_grid_metadata(grid, "experiment");
+    pineappl_string_delete(comment);
+    pineappl_string_delete(experiment);
+
     // ---
     // Write the grid to disk - the filename can be anything ...
     std::string filename = "sidis-toygrid.pineappl.lz4";
diff --git a/examples/fortran/dyaa.f90 b/examples/fortran/dyaa.f90
@@ -109,18 +109,18 @@ program dyaa
         !   above in 'channels')
         ! - for PDF parameters 'x1, x2, q2',
         ! - with the given 'weight'
-        call pineappl_grid_fill2(grid, order_idx, abs(yll), channel_idx, [ x1, x2, q2 ], weight)
+        call pineappl_grid_fill2(grid, order_idx, abs(yll), channel_idx, [ q2, x1, x2 ], weight)
     end do
 
     ! set metadata - this isn't strictly needed, but usually useful (plot script, ...)
-    call pineappl_grid_set_key_value(grid, 'arxiv', '1310.7291')
-    call pineappl_grid_set_key_value(grid, 'description', 'CMS double-differential Drell—Yan cross section at 7 TeV')
-    call pineappl_grid_set_key_value(grid, 'hepdata', '10.17182/hepdata.62207.v1/t8')
-    call pineappl_grid_set_key_value(grid, 'x1_label', 'yll')
-    call pineappl_grid_set_key_value(grid, 'x1_label_tex', '$y_{\ell\bar{\ell}}$')
+    call pineappl_grid_set_metadata(grid, 'arxiv', '1310.7291')
+    call pineappl_grid_set_metadata(grid, 'description', 'CMS double-differential Drell—Yan cross section at 7 TeV')
+    call pineappl_grid_set_metadata(grid, 'hepdata', '10.17182/hepdata.62207.v1/t8')
+    call pineappl_grid_set_metadata(grid, 'x1_label', 'yll')
+    call pineappl_grid_set_metadata(grid, 'x1_label_tex', '$y_{\ell\bar{\ell}}$')
     ! rapidity doesn't have a unit (other observables could be GeV, TeV, ...)
-    call pineappl_grid_set_key_value(grid, 'x1_unit', '')
-    call pineappl_grid_set_key_value(grid, 'y_unit', 'pb')
+    call pineappl_grid_set_metadata(grid, 'x1_unit', '')
+    call pineappl_grid_set_metadata(grid, 'y_unit', 'pb')
 
     ! optimize the grid representation (makes the file smaller)
     call pineappl_grid_optimize(grid)
diff --git a/examples/fortran/pineappl.f90 b/examples/fortran/pineappl.f90
@@ -173,7 +173,7 @@ subroutine channels_add(channels, combinations, pdg_id_combinations, factors) &
 
         type (c_ptr) function channels_new(convolutions) bind(c, name = 'pineappl_channels_new')
             use iso_c_binding
-            integer (c_int32_t), value :: convolutions
+            integer (c_size_t), value :: convolutions
         end function
 
         integer (c_size_t) function grid_bin_count(grid) bind(c, name = 'pineappl_grid_bin_count')
@@ -271,6 +271,13 @@ function grid_key_value(grid, key) bind(c, name = 'pineappl_grid_key_value')
             type (c_ptr)        :: grid_key_value
         end function
 
+        function grid_metadata(grid, key) bind(c, name = 'pineappl_grid_metadata')
+            use iso_c_binding
+            type (c_ptr), value :: grid
+            character (c_char)  :: key(*)
+            type (c_ptr)        :: grid_metadata
+        end function
+
         function grid_channels(grid) bind(c, name = 'pineappl_grid_channels')
             use iso_c_binding
             type (c_ptr), value :: grid
@@ -358,6 +365,12 @@ subroutine grid_set_key_value(grid, key, valju) bind(c, name = 'pineappl_grid_se
             character (c_char)  :: key(*), valju(*)
         end subroutine
 
+        subroutine grid_set_metadata(grid, key, valju) bind(c, name = 'pineappl_grid_set_metadata')
+            use iso_c_binding
+            type (c_ptr), value :: grid
+            character (c_char)  :: key(*), valju(*)
+        end subroutine
+
         subroutine grid_set_remapper(grid, dimensions, normalizations, limits) bind(c, name = 'pineappl_grid_set_remapper')
             use iso_c_binding
             type (c_ptr), value       :: grid
@@ -434,9 +447,9 @@ function c_f_string(c_str) result(f_str)
     type (pineappl_channels) function pineappl_channels_new(convolutions)
         implicit none
 
-        integer (c_int32_t), value :: convolutions
+        integer, intent(in) :: convolutions
 
-        pineappl_channels_new = pineappl_channels(channels_new(convolutions))
+        pineappl_channels_new = pineappl_channels(channels_new(int(convolutions, c_size_t)))
     end function
 
     integer function pineappl_grid_bin_count(grid)
@@ -538,7 +551,7 @@ function pineappl_grid_convolve(grid, xfx, alphas, pdfs_state, alphas_state, ord
             alphas_state, &
             [(logical(order_mask(i), c_bool), i = 1, size(order_mask))], &
             [(logical(channel_mask(i), c_bool), i = 1, size(channel_mask))], &
-            [(int(bin_indices, c_size_t), i = 1, size(bin_indices))], &
+            [(int(bin_indices(i), c_size_t), i = 1, size(bin_indices))], &
             int(nb_scales, c_size_t), &
             mu_scales, &
             res &
@@ -605,6 +618,18 @@ function pineappl_grid_key_value(grid, key) result(res)
         res = c_f_string(grid_key_value(grid%ptr, key // c_null_char))
     end function
 
+    function pineappl_grid_metadata(grid, key) result(res)
+        use iso_c_binding
+
+        implicit none
+
+        type (pineappl_grid), intent(in) :: grid
+        character (*), intent(in)        :: key
+        character (:), allocatable       :: res
+
+        res = c_f_string(grid_metadata(grid%ptr, key // c_null_char))
+    end function
+
     type (pineappl_channels) function pineappl_grid_channels(grid)
         implicit none
 
@@ -752,6 +777,17 @@ subroutine pineappl_grid_set_key_value(grid, key, valju)
         call grid_set_key_value(grid%ptr, key // c_null_char, valju // c_null_char)
     end subroutine
 
+    subroutine pineappl_grid_set_metadata(grid, key, valju)
+        use iso_c_binding
+
+        implicit none
+
+        type (pineappl_grid), intent(in) :: grid
+        character (*), intent(in)        :: key, valju
+
+        call grid_set_metadata(grid%ptr, key // c_null_char, valju // c_null_char)
+    end subroutine
+
     subroutine pineappl_grid_set_remapper(grid, dimensions, normalizations, limits)
         use iso_c_binding
 
@@ -788,10 +824,10 @@ subroutine pineappl_channels_add(channels, combinations, pdg_id_combinations, fa
 
         implicit none
 
-        type (pineappl_channels), intent(in)             :: channels
-        integer, intent(in)                              :: combinations
-        integer, dimension(2 * combinations), intent(in) :: pdg_id_combinations
-        real (dp), dimension(combinations), intent(in)   :: factors
+        type (pineappl_channels), intent(in)           :: channels
+        integer, intent(in)                            :: combinations
+        integer, dimension(*), intent(in)              :: pdg_id_combinations
+        real (dp), dimension(combinations), intent(in) :: factors
 
         call channels_add(channels%ptr, int(combinations, c_size_t), pdg_id_combinations, factors)
     end subroutine
diff --git a/examples/fortran/test.f90 b/examples/fortran/test.f90
@@ -247,6 +247,13 @@ program test_pineappl
 
     call pineappl_grid_set_key_value(grid, "set_key_value", "set_key_value: success")
 
+    call pineappl_grid_set_metadata(grid, "set_metadata", "set_metadata: success")
+
+    if (pineappl_grid_metadata(grid, "set_metadata") /= "set_metadata: success") then
+        write(*, *) "pineappl_grid_metadata(): '", pineappl_grid_metadata(grid, "set_metadata"), "'"
+        error stop "error: pineappl_grid_metadata"
+    end if
+
     ! NOTE: At this point we have the bins: [0, 1, 2, 3]
     call pineappl_grid_set_remapper(grid, 2, [1.0_dp, 1.0_dp, 1.0_dp], &
         [0.0_dp, 1.0_dp, 10.0_dp, 11.0_dp, 1.0_dp, 3.0_dp, 11.0_dp, 13.0_dp, 15.0_dp, 20.0_dp])
diff --git a/examples/object-oriented-cpp/PineAPPL.hpp b/examples/object-oriented-cpp/PineAPPL.hpp
@@ -229,18 +229,18 @@ struct Grid {
    * @param key key
    * @param value value
    */
-  void set_key_value(const std::string &key, const std::string &value) const {
-    pineappl_grid_set_key_value(this->raw, key.c_str(), value.c_str());
+  void set_metadata(const std::string &key, const std::string &value) const {
+    pineappl_grid_set_metadata(this->raw, key.c_str(), value.c_str());
   }
 
   /**
    * @brief Get a metadata entry.
    * @param key key
    * @return value
    */
-  std::string get_key_value(const std::string &key) const {
-    auto *value = pineappl_grid_key_value(this->raw, key.c_str());
-    std::string res(value);
+  std::string get_metadata(const std::string &key) const {
+    auto *value = pineappl_grid_metadata(this->raw, key.c_str());
+    std::string res(value != nullptr ? value : "");
     // delete the allocated object
     pineappl_string_delete(value);
     return res;
diff --git a/examples/object-oriented-cpp/dyaa.cpp b/examples/object-oriented-cpp/dyaa.cpp
@@ -199,10 +199,10 @@ int main() {
   }
 
   // store some metadata in the grid
-  grid.set_key_value("events", "10000000");
+  grid.set_metadata("events", "10000000");
 
   // read out the stored value and print it on stdout
-  const auto value = grid.get_key_value("events");
+  const auto value = grid.get_metadata("events");
   std::printf("Finished running %s events.\n", value.c_str());
 
   // write the grid to disk - with `.lz4` suffix the grid is automatically LZ4
diff --git a/maintainer/download-test-data.sh b/maintainer/download-test-data.sh
@@ -52,6 +52,7 @@ files=(
     'https://data.nnpdf.science/pineappl/test-data/ZEUS_2JET_319GEV_374PB-1_DIF_ETQ2_BIN6.tar'
     'https://data.nnpdf.science/pineappl/test-data/LHCB_WP_8TEV.pineappl.lz4'
     'https://data.nnpdf.science/pineappl/test-data/LHCB_WP_8TEV.tar'
+    'https://data.nnpdf.science/pineappl/test-data/test_newton_convergence.pineappl.lz4'
     'https://ploughshare.web.cern.ch/ploughshare/db/applfast/applfast-atlas-dijets-fnlo-arxiv-1312.3524/grids/applfast-atlas-dijets-fnlo-arxiv-1312.3524-xsec000.tab.gz'
     'https://ploughshare.web.cern.ch/ploughshare/db/applfast/applfast-h1-dijets-appl-arxiv-0010054/grids/applfast-h1-dijets-appl-arxiv-0010054-xsec000.appl'
     'https://ploughshare.web.cern.ch/ploughshare/db/applfast/applfast-h1-incjets-fnlo-arxiv-0706.3722/grids/applfast-h1-incjets-fnlo-arxiv-0706.3722-xsec000.tab.gz'
diff --git a/pineappl/src/interpolation.rs b/pineappl/src/interpolation.rs
@@ -20,7 +20,7 @@ mod applgrid {
         let mut yp = y;
         let mut deltap = f64::INFINITY;
 
-        for _ in 0..10 {
+        for _ in 0..15 {
             let x = (-yp).exp();
             let delta = (1.0 - x).mul_add(-5.0, y - yp);
             if (delta == 0.0) || ((delta.abs() < 2e-15) && (delta.abs() >= deltap.abs())) {
@@ -804,4 +804,14 @@ mod tests {
             ulps = 4
         );
     }
+
+    #[test]
+    fn issue_372() {
+        assert_approx_eq!(
+            f64,
+            applgrid::fx2(3.751520963950722),
+            0.4221667753589648,
+            ulps = 4
+        );
+    }
 }
diff --git a/pineappl_capi/src/lib.rs b/pineappl_capi/src/lib.rs
@@ -1860,6 +1860,54 @@ pub unsafe extern "C" fn pineappl_channels_entry(
         .for_each(|(from, to)| *to = *from);
 }
 
+/// Return the value for `key` stored in `grid`. If `key` isn't found, `NULL` will be returned.
+/// After usage the string must be deallocated using [`pineappl_string_delete`].
+///
+/// # Safety
+///
+/// If `grid` does not point to a valid `Grid` object, for example when `grid` is the `NULL`
+/// pointer, this function is not safe to call. The parameter `key` must be non-`NULL` and a valid
+/// C string.
+#[no_mangle]
+#[must_use]
+pub unsafe extern "C" fn pineappl_grid_metadata(
+    grid: *const Grid,
+    key: *const c_char,
+) -> *mut c_char {
+    let grid = unsafe { &*grid };
+    let key = unsafe { CStr::from_ptr(key) }.to_string_lossy();
+
+    if let Some(value) = grid.metadata().get(key.as_ref()) {
+        CString::new(value.as_str()).unwrap().into_raw()
+    } else {
+        std::ptr::null_mut()
+    }
+}
+
+/// Sets an internal key-value pair for the grid.
+///
+/// # Safety
+///
+/// If `grid` does not point to a valid `Grid` object, for example when `grid` is the null pointer,
+/// this function is not safe to call. The parameters `key` and `value` must be non-`NULL` and
+/// valid C strings.
+#[no_mangle]
+pub unsafe extern "C" fn pineappl_grid_set_metadata(
+    grid: *mut Grid,
+    key: *const c_char,
+    value: *const c_char,
+) {
+    let grid = unsafe { &mut *grid };
+    let key = unsafe { CStr::from_ptr(key) }
+        .to_string_lossy()
+        .into_owned();
+    let value = unsafe { CStr::from_ptr(value) }
+        .to_string_lossy()
+        .into_owned();
+
+    grid.metadata_mut().insert(key, value);
+}
+
 /// An extension of `pineappl_grid_order_params` that accounts for the order of the fragmentation
 /// logs.
 ///
diff --git a/pineappl_cli/tests/write.rs b/pineappl_cli/tests/write.rs
@@ -682,6 +682,23 @@ fn optimize() {
         .stdout("");
 }
 
+#[test]
+fn optimize_newton_convergence() {
+    let output = NamedTempFile::new("optimized.pineappl.lz4").unwrap();
+
+    Command::cargo_bin("pineappl")
+        .unwrap()
+        .args([
+            "write",
+            "--optimize",
+            "../test-data/test_newton_convergence.pineappl.lz4",
+            output.path().to_str().unwrap(),
+        ])
+        .assert()
+        .success()
+        .stdout("");
+}
+
 #[test]
 fn set_bins() {
     let output = NamedTempFile::new("set_bins.pineappl.lz4").unwrap();
