Skip to content

[build] Update UNURAN to 1.11.0 #18747

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

Merged
merged 2 commits into from
May 20, 2025

Conversation

vepadulano
Copy link
Member

Update builtin to latest version, including a crucial fix introduced by commit
unuran/unuran@8cb1dc4 which avoids the following build error visible with a GCC 15 build:

In file included from pinv.c:553:
pinv_newton.ch: In function '_unur_pinv_newton_maxerror':
pinv_newton.ch:581:9: error: too many arguments to function '_unur_pinv_cubic_hermite_is_monotone'; expected 0, have 4
  581 |       ! _unur_pinv_cubic_hermite_is_monotone(gen,ui,zi,xval))
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~
pinv.c:443:12: note: declared here
  443 | static int _unur_pinv_cubic_hermite_is_monotone();
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pinv_newton.ch: At top level:
pinv_newton.ch:669:1: error: conflicting types for '_unur_pinv_cubic_hermite_is_monotone'; have 'int(struct unur_gen *, double *, double *, double *)'
  669 | _unur_pinv_cubic_hermite_is_monotone(struct unur_gen *gen, double *ui, double *zi, double *xval)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pinv.c:443:12: note: previous declaration of '_unur_pinv_cubic_hermite_is_monotone' with type 'int(void)'
  443 | static int _unur_pinv_cubic_hermite_is_monotone();
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This also removes the need for a patch of the LCG build system https://gitlab.cern.ch/sft/lcgcmake/-/commit/7c93a4a2747d01333e739f67f43f7cd8c607dc34

@vepadulano vepadulano requested a review from dpiparo May 16, 2025 13:00
@vepadulano vepadulano self-assigned this May 16, 2025
@vepadulano vepadulano requested a review from lmoneta as a code owner May 16, 2025 13:00
@vepadulano vepadulano added in:Build System clean build Ask CI to do non-incremental build on PR labels May 16, 2025
@vepadulano vepadulano closed this May 16, 2025
@vepadulano vepadulano reopened this May 16, 2025
Copy link

github-actions bot commented May 16, 2025

Test Results

    18 files      18 suites   3d 6h 56m 37s ⏱️
 2 744 tests  2 622 ✅   0 💤 122 ❌
48 201 runs  47 879 ✅ 200 💤 122 ❌

For more details on these failures, see this check.

Results for commit a5276e6.

♻️ This comment has been updated with latest results.

@vepadulano vepadulano requested a review from bellenot as a code owner May 16, 2025 13:41
Update builtin to latest version, including a crucial fix introduced by
commit
unuran/unuran@8cb1dc4
which avoids the following build error visible with a GCC 15 build:
```
In file included from pinv.c:553:
pinv_newton.ch: In function '_unur_pinv_newton_maxerror':
pinv_newton.ch:581:9: error: too many arguments to function '_unur_pinv_cubic_hermite_is_monotone'; expected 0, have 4
  581 |       ! _unur_pinv_cubic_hermite_is_monotone(gen,ui,zi,xval))
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~
pinv.c:443:12: note: declared here
  443 | static int _unur_pinv_cubic_hermite_is_monotone();
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pinv_newton.ch: At top level:
pinv_newton.ch:669:1: error: conflicting types for '_unur_pinv_cubic_hermite_is_monotone'; have 'int(struct unur_gen *, double *, double *, double *)'
  669 | _unur_pinv_cubic_hermite_is_monotone(struct unur_gen *gen, double *ui, double *zi, double *xval)
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
pinv.c:443:12: note: previous declaration of '_unur_pinv_cubic_hermite_is_monotone' with type 'int(void)'
  443 | static int _unur_pinv_cubic_hermite_is_monotone();
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

This also removes the need for a patch of the LCG build system
https://gitlab.cern.ch/sft/lcgcmake/-/commit/7c93a4a2747d01333e739f67f43f7cd8c607dc34
@vepadulano vepadulano force-pushed the update-builtin-unuran branch from 88309ae to a47f69c Compare May 19, 2025 06:54
@guitargeek
Copy link
Contributor

I'm curious @andresailer @vepadulano, why is ROOT on LCG using builtin_unuran? Doesn't the LCG build generally try to avoid builtins?

@andresailer
Copy link
Contributor

Hi @guitargeek ,

this seems to go back 12 years, I don't know why we started with it. We would like to avoid adding a dependency from our stack to ROOT so that downstream customers of ROOT don't have to adapt to find more external packages.

@vepadulano
Copy link
Member Author

The updated builtin doesn't seem to break any platform. I don't know what could be the cause of the Windows failures since this PR is not using the builtin on Windows. Before merging the last two commits will have to be removed

Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, please remove the ci modifications before merging.

@vepadulano vepadulano force-pushed the update-builtin-unuran branch from a5276e6 to a2c86d3 Compare May 20, 2025 14:40
@vepadulano vepadulano merged commit 911b157 into root-project:master May 20, 2025
19 of 22 checks passed
@dkonst13
Copy link

@vepadulano , @dpiparo , Many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean build Ask CI to do non-incremental build on PR in:Build System
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants