Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[semigroups] Update to 5.3.4 #864

Merged
merged 1 commit into from
Jan 25, 2024
Merged

[semigroups] Update to 5.3.4 #864

merged 1 commit into from
Jan 25, 2024

Conversation

gap-package-distribution-bot[bot]
Copy link
Contributor

@gap-package-distribution-bot gap-package-distribution-bot bot commented Jan 5, 2024 •
edited
Loading

@gap-package-distribution-bot gap-package-distribution-bot bot added automated pr Automatically applied to PRs created by a GH workflow package update labels Jan 5, 2024
@gap-package-distribution-bot GAP Package Distribution Bot
Copy link
Contributor Author

gap-package-distribution-bot bot commented Jan 5, 2024 •
edited
Loading

Package Evaluation Report for GAP master

Job Properties

Testing: master/2024-01-24-18:49:46-324fdbf2 vs master/2024-01-24-17:46:19-fd4c4092

Generated by Workflow: https://github.com/gap-system/PackageDistro/actions/runs/7644215748

In total, 157 packages were tested, out of which 153 succeeded, 0 failed and 4 were skipped.

✔️ Packages still succeeding

153 package(s) succeeded tests also on the previous version.

Click to show packages!

➖ Packages that were skipped

4 package(s) skipped tests also on the previous version.

Click to show packages!

@fingolfin
Copy link
Member

@james-d-mitchell new Semigroups package fails to compile with hundreds of errors of the form

  In file included from ./bin/include/libsemigroups/HPCombi/include/epu.hpp:715,
  from ./bin/include/libsemigroups/HPCombi/include/hpcombi.hpp:19,
  from ./bin/include/libsemigroups/hpcombi.hpp:35,
  from ./bin/include/libsemigroups/transf.hpp:49,
  from src/to_cpp.hpp:61,
  from src/froidure-pin-bipart.cpp:21:
  ./bin/include/libsemigroups/HPCombi/include/epu_impl.hpp: In instantiation of ‘HPCombi::epu8 HPCombi::network_sort_perm(HPCombi::epu8&, std::array<__vector(16) unsigned char, sz>) [with bool Increassing = true; long unsigned int sz = 6; HPCombi::epu8 = __vector(16) unsigned char]’:
  ./bin/include/libsemigroups/HPCombi/include/epu_impl.hpp:200:54:   required from here
  ./bin/include/libsemigroups/HPCombi/include/epu_impl.hpp:132:31: error: cannot convert ‘HPCombi::epu8’ {aka ‘__vector(16) unsigned char’} to ‘__m128i’ {aka ‘__vector(2) long long int’}
  132 |         res = _mm_blendv_epi8(res, permuted(res, round), cmp);
  |                               ^~~
  |                               |
  |                               HPCombi::epu8 {aka __vector(16) unsigned char}
  In file included from /usr/lib/gcc/x86_64-linux-gnu/9/include/immintrin.h:37,
  from /usr/lib/gcc/x86_64-linux-gnu/9/include/x86intrin.h:32,
  from ./bin/include/libsemigroups/HPCombi/include/epu.hpp:25,
  from ./bin/include/libsemigroups/HPCombi/include/hpcombi.hpp:19,
  from ./bin/include/libsemigroups/hpcombi.hpp:35,
  from ./bin/include/libsemigroups/transf.hpp:49,
  from src/to_cpp.hpp:61,
  from src/froidure-pin-bipart.cpp:21:
  /usr/lib/gcc/x86_64-linux-gnu/9/include/smmintrin.h:179:26: note:   initializing argument 1 of ‘__m128i _mm_blendv_epi8(__m128i, __m128i, __m128i)’
  179 | _mm_blendv_epi8 (__m128i __X, __m128i __Y, __m128i __M)
  |                  ~~~~~~~~^~~

@james-d-mitchell
Copy link
Collaborator

Interesting, this can be overcome temporarily by passing the configure flag --disable-hpcombi.

It looks like the flag -flax-vector-conversions isn't being set/passed properly.

@gap-package-distribution-bot gap-package-distribution-bot bot force-pushed the automatic/semigroups branch 2 times, most recently from e8929ce to fb6960b Compare January 6, 2024 16:09
@fingolfin
Copy link
Member

So this code is using -mavx it seems ... Isn't this like the use of -march=native, where it generates binaries that only run on some hosts but not all because they use CPU extensions not available everywhere?

Anyway, how do we proceed? Modifying the testing setup here to pass --disable-hpcombi does not really solve anything because endusers may still run into this issue. We could teach GAP's bin/BuildPackages.sh to pass it, but that would then affect all versions of Semigroups unconditionally.

@fingolfin
Copy link
Member

For the record, I also get those errors on two different x86 Linux systems (one with Ubuntu, one with Arch).

@james-d-mitchell
Copy link
Collaborator

So this code is using -mavx it seems ... Isn't this like the use of -march=native, where it generates binaries that only run on some hosts but not all because they use CPU extensions not available everywhere?

Anyway, how do we proceed? Modifying the testing setup here to pass --disable-hpcombi does not really solve anything because endusers may still run into this issue. We could teach GAP's bin/BuildPackages.sh to pass it, but that would then affect all versions of Semigroups unconditionally.

I'll investigate, something must be wrong with the build system, I'll try to figure it out and make a new release.

@gap-package-distribution-bot gap-package-distribution-bot bot force-pushed the automatic/semigroups branch 4 times, most recently from 725ca92 to 97a3458 Compare January 12, 2024 14:15
@gap-package-distribution-bot gap-package-distribution-bot bot force-pushed the automatic/semigroups branch 5 times, most recently from c9250e7 to 8f8c60b Compare January 22, 2024 13:10
@fingolfin fingolfin added the skip tests No CI tests are run for PRs with this label. Otherwise they run each time the distro is updated label Jan 22, 2024
@fingolfin
Copy link
Member

I've added the "skip tests" label to this PR for now, as it doesn't help to retest the same semigroups release each time some other package is updated. Just need to remove it once there is an actual bug fix update (so if in doubt, please ping me when you make a new release, @james-d-mitchell )

@gap-package-distribution-bot gap-package-distribution-bot bot force-pushed the automatic/semigroups branch 6 times, most recently from 0cd8bad to 8632ed2 Compare January 23, 2024 21:08
@fingolfin fingolfin added skip tests No CI tests are run for PRs with this label. Otherwise they run each time the distro is updated and removed skip tests No CI tests are run for PRs with this label. Otherwise they run each time the distro is updated labels Jan 24, 2024
@james-d-mitchell
Copy link
Collaborator

@fingolfin new version of Semigroups released that should fix the issues highlighted hre.

@fingolfin fingolfin removed the skip tests No CI tests are run for PRs with this label. Otherwise they run each time the distro is updated label Jan 24, 2024
@gap-package-distribution-bot gap-package-distribution-bot bot changed the title [semigroups] Update to 5.3.3 [semigroups] Update to 5.3.4 Jan 24, 2024
@fingolfin fingolfin merged commit 52e8867 into main Jan 25, 2024
311 of 312 checks passed
@fingolfin fingolfin deleted the automatic/semigroups branch January 25, 2024 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
automated pr Automatically applied to PRs created by a GH workflow package update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@fingolfin @james-d-mitchell