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.

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

Simpcomp (was: Union sometimes fails if applied to a list of lists whose representations are mixed between PlistRep and RangeRep) #1447

Closed
markusbaumeister opened this issue Jun 23, 2017 · 7 comments
Labels
kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them kind: bug Issues describing general bugs, and PRs fixing them regression A bug that only occurs in the branch, not in a release topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker)
Milestone

Comments

@markusbaumeister
Copy link
Contributor

Observed behaviour

If Union gets a list of PlistRep-lists and RangeRep-lists, it sometimes does not return the union of those collections.

Input:
Union( [ [1..2], [17], [20], [11,23..23], [14,26..26], [1,17..17], [23] ] );
Union( [ [1..2], [5,6], [5,11], [6,14..14], [17], [20], [11,23..23], [14,26..26], [1,17..17], [2,20..20], [23], [26] ] );
Output:
[1,2]
[ 1, 2, 5, 6, 11 ]

Expected behaviour

Union should return the union of the appropriate collections. In the given example the results should be respectively:
[1,2,11,14,17,20,23,26]
[ 1, 2, 5, 6, 11, 14, 17, 20, 23, 26 ]

Copy and paste GAP banner (to tell us about your setup)

This was tested on three different versions of GAP (up to 4.8.7).

┌───────┐ GAP 4.8.3, 19-Mar-2016, build of 2016-06-09 11:32:41 (CEST)
│ GAP │ http://www.gap-system.org
└───────┘ Architecture: x86_64-pc-linux-gnu-gcc-default64

┌───────┐ GAP 4.8.5, 25-Sep-2016, build of 2016-09-30 22:20:20 (CEST)
│ GAP │ http://www.gap-system.org
└───────┘ Architecture: x86_64-pc-linux-gnu-gcc-default64

┌───────┐ GAP 4.8.7, 24-Mar-2017, build of 2017-05-16 12:12:16 (UTC)
│ GAP │ https://www.gap-system.org
└───────┘ Architecture: x86_64-pc-linux-gnu-gcc-default64

@markuspf
Copy link
Member

Note that this is fixed in the master branch, though I fail to find the commit that does it right now.

@markuspf
Copy link
Member

I suspect it is this one: #444

@fingolfin fingolfin added the kind: bug Issues describing general bugs, and PRs fixing them label Jul 3, 2017
@fingolfin
Copy link
Member

Perhaps @stevelinton can have a look at this, given that he rewrote the Union code and hence is probably most familiar with it?

@fingolfin fingolfin added the regression A bug that only occurs in the branch, not in a release label Jul 3, 2017
@fingolfin fingolfin added this to the GAP 4.9.0 milestone Jul 3, 2017
@fingolfin fingolfin added the kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them label Jul 3, 2017
@stevelinton
Copy link
Contributor

This is fixed in master and so will be fixed in 4.9.0. The only question is whether it's worth fixing on the stable branch for 4.8.8 &c. If so, are we better to move the fairly large, but hopefully by now reasonably well tested solution that is in master, or to try and write a minimal fix for the bug? The big fix does require a minor fix to simpcomp -- maybe that's already released.

@fingolfin
Copy link
Member

@stevelinton thanks for the update. Looking at #444, there was a commit by @simpcomp-team, see simpcomp-team/simpcomp@a06ed18 -- I assume this resolves the issue, but there hasn't been a simpcomp release since then. I filed simpcomp-team/simpcomp#5 on this

@olexandr-konovalov olexandr-konovalov added the topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker) label Sep 11, 2017
@olexandr-konovalov olexandr-konovalov changed the title Union sometimes fails if applied to a list of lists whose representations are mixed between PlistRep and RangeRep Simpcomp (was: Union sometimes fails if applied to a list of lists whose representations are mixed between PlistRep and RangeRep) Sep 11, 2017
@olexandr-konovalov
Copy link
Member

Simpcomp 2.1.7 released and picked up for the redistribution (see simpcomp-team/simpcomp#5). It should appear in GAP 4.9.0 archive.

@olexandr-konovalov
Copy link
Member

Simpcomp 2.1.7 has been included in GAP 4.8.10 distribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: wrong result Issues describing bugs that result in mathematically or otherwise wrong results, and PRs fixing them kind: bug Issues describing general bugs, and PRs fixing them regression A bug that only occurs in the branch, not in a release topic: packages issues or PRs related to package handling, or specific to a package (for packages w/o issue tracker)
Projects
None yet
Development

No branches or pull requests

5 participants