paraview: patch catalyst etc. to build with oneapi by hppritcha · Pull Request #33562 · spack/spack

hppritcha · 2022-10-27T14:57:34Z

without this patch, build of paraview has a meltdown when reaching 3rd party catalyst and other packages with these types of errors:

335 /tmp/foo/spack-stage/spack-stage-paraview-5.10.1-gscoqxhhakjyyfirdefuhmi2bzw4scho/spack-src/VTK/ThirdParty/fmt/vtkfmt/vtkfmt/format.h:1732:11: error: cannot capture a bi
t-field by reference
336 if (sign) *it++ = static_cast(data::signs[sign]);
337 ^

Signed-off-by: Howard Pritchard [email protected]

vicentebolea

Thanks for the contribution, this is indeed helpful.

Find the requested changes and please file the same changes upstream at https://github.com/fmtlib/fmt

vicentebolea · 2022-10-27T16:11:00Z

var/spack/repos/builtin/packages/paraview/catalyst-etc_oneapi_fix.patch

+       isinf ? (fspecs.upper ? "INF" : "inf") : (fspecs.upper ? "NAN" : "nan");
+   constexpr size_t str_size = 3;
+-  auto sign = fspecs.sign;
+  auto sign = (uint)fspecs.sign;


Please change (uint) -> static_cast<unsigned int>() for wider portability

vicentebolea · 2022-10-27T16:16:26Z

var/spack/repos/builtin/packages/paraview/catalyst-etc_oneapi_fix.patch

+   int significand_size = get_significand_size(fp);
+   static const Char zero = static_cast<Char>('0');
+-  auto sign = fspecs.sign;
+  auto sign = (uint)fspecs.sign;


vicentebolea · 2022-10-27T16:16:33Z

var/spack/repos/builtin/packages/paraview/catalyst-etc_oneapi_fix.patch

+       isinf ? (fspecs.upper ? "INF" : "inf") : (fspecs.upper ? "NAN" : "nan");
+   constexpr size_t str_size = 3;
+-  auto sign = fspecs.sign;
+  auto sign = (uint)fspecs.sign;


vicentebolea · 2022-10-27T16:16:40Z

var/spack/repos/builtin/packages/paraview/catalyst-etc_oneapi_fix.patch

+   int significand_size = get_significand_size(fp);
+   static const Char zero = static_cast<Char>('0');
+-  auto sign = fspecs.sign;
+  auto sign = (uint)fspecs.sign;


hppritcha · 2022-10-27T21:39:05Z

the patch doesn't work on head of master on https://github.com/fmtlib/fmt namely doesn't compile.

vicentebolea · 2022-10-31T23:25:20Z

I see, the current master of fmt has changed so that this does not apply anymore. @hppritcha could you share the spec invocation where you observed the error so that I can replicate this. Thanks!

hppritcha · 2022-11-01T22:16:27Z

==> Concretized paraview%[email protected] ^perl%gcc ^[email protected] ^rust%gcc
[+] kv37cce [email protected]%[email protected] cflags="-fPIC -Wl,--allow-multiple-definition -Qunused-arguments -Wl,-undefined,dynamic_lookup" cxxflags="-fPIC -Wl,--allow-multiple-definition -Wl,-undefined,dynamic_lookup -Qunused-arguments -std=c++14 -stdlib=libstdc++ -L/projects/opt/centos8/x86_64/gcc/11.2.0/lib64 -Wl,-rpath,/projects/opt/centos8/x86_64/gcc/11.2.0/lib64" ~~adios2~~advanced_debug~~cuda+development_files~~examples~~eyedomelighting~~fortran+hdf5~~ipo+kits+mpi+opengl2~~osmesa~~pagosa~~python+python3~qt+shared build_edition=catalyst_rendering build_type=RelWithDebInfo patches=730539b,acb3805 use_vtkm=off arch=linux-rhel8-skylake

vicentebolea · 2022-11-24T20:03:51Z

@hppritcha I cannot replicate the error but I used oneapi2022 (not 23) and perhaps a different GCC version. One thing that I can state is that it does not break the build and it only applies to oneapi builds for 5.10.0-1.

As the author noticed, this is not a problem in ParaView > 5.10.1 since Catalyst is not included in PAraView Source Code Dist anymore.

vicentebolea · 2022-11-24T20:04:24Z

@hppritcha it has been a while please rebase to kick off the CI.

bernhardkaindl

@vicentebolea Because you approved and requested a re-run of the pipeline by a rebase, I've merged the changes from develop to do this.

(spack maintainers have a button for this from github which works fine),
should @hppritcha still want to make a rebase, but github will do I when it squashes and rebases automatically for the merge into develop, so it should be taken care of by github after CI has passed again.

I will also enable auto-rebase & merge to develop since as far as I can see you approved this PR.

@hppritcha @vicentebolea In case the gitlab-ci CI action fails for some unrelated spurious reason, please ping @bernhardkaindl with a comment (or in case I miss your ping, other reviewers/maintainers)

vicentebolea · 2022-12-20T23:51:50Z

@hppritcha please rebase again to re-trigger the CI.

without this patch, build of paraview has a meltdown when reaching 3rd party catalyst and other packages with these types of errors: 335 /tmp/foo/spack-stage/spack-stage-paraview-5.10.1-gscoqxhhakjyyfirdefuhmi2bzw4scho/spack-src/VTK/ThirdParty/fmt/vtkfmt/vtkfmt/format.h:1732:11: error: cannot capture a bi t-field by reference 336 if (sign) *it++ = static_cast<Char>(data::signs[sign]); 337 ^ Signed-off-by: Howard Pritchard <[email protected]>

hppritcha · 2022-12-21T18:31:27Z

@vicentebolea rebased

without this patch, build of paraview has a meltdown when reaching 3rd party catalyst and other packages with these types of errors: 335 /tmp/foo/spack-stage/spack-stage-paraview-5.10.1-gscoqxhhakjyyfirdefuhmi2bzw4scho/spack-src/VTK/ThirdParty/fmt/vtkfmt/vtkfmt/format.h:1732:11: error: cannot capture a bi t-field by reference 336 if (sign) *it++ = static_cast<Char>(data::signs[sign]); 337 ^ Signed-off-by: Howard Pritchard <[email protected]> Signed-off-by: Howard Pritchard <[email protected]>

spackbot-app bot added patch update-package labels Oct 27, 2022

spackbot-app bot requested review from chuckatkins, danlipsa and vicentebolea October 27, 2022 14:57

hppritcha force-pushed the paraview_catalyst_oneapi_fix branch 2 times, most recently from 2e071c0 to 5c698ce Compare October 27, 2022 15:30

eugeneswalker added the e4s label Oct 27, 2022

vicentebolea suggested changes Oct 27, 2022

View reviewed changes

hppritcha force-pushed the paraview_catalyst_oneapi_fix branch from 5c698ce to 57aa83f Compare October 27, 2022 20:43

tldahlgren assigned vicentebolea Oct 31, 2022

vicentebolea previously approved these changes Nov 24, 2022

View reviewed changes

bernhardkaindl previously approved these changes Nov 25, 2022

View reviewed changes

bernhardkaindl enabled auto-merge (squash) November 25, 2022 23:10

auto-merge was automatically disabled December 21, 2022 18:30
Head branch was pushed to by a user without write access

hppritcha dismissed stale reviews from bernhardkaindl and vicentebolea via 1502572 December 21, 2022 18:30

hppritcha force-pushed the paraview_catalyst_oneapi_fix branch from e85f44a to 1502572 Compare December 21, 2022 18:30

vicentebolea approved these changes Dec 21, 2022

View reviewed changes

alalazo approved these changes Dec 21, 2022

View reviewed changes

alalazo enabled auto-merge (squash) December 21, 2022 18:59

alalazo merged commit c74bbc6 into spack:develop Dec 22, 2022

Conversation

hppritcha commented Oct 27, 2022

Uh oh!

vicentebolea left a comment

Choose a reason for hiding this comment

Uh oh!

vicentebolea Oct 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicentebolea Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

vicentebolea Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

vicentebolea Oct 27, 2022

Choose a reason for hiding this comment

Uh oh!

hppritcha commented Oct 27, 2022

Uh oh!

vicentebolea commented Oct 31, 2022

Uh oh!

hppritcha commented Nov 1, 2022

Uh oh!

vicentebolea commented Nov 24, 2022

Uh oh!

vicentebolea commented Nov 24, 2022

Uh oh!

bernhardkaindl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vicentebolea commented Dec 20, 2022

Uh oh!

hppritcha commented Dec 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vicentebolea Oct 27, 2022 •

edited

Loading

bernhardkaindl left a comment •

edited

Loading