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

category of lazy family wrong if infinite combinatorial class used as index #18849

Open
cnassau opened this issue Jul 3, 2015 · 17 comments
Open

Comments

@cnassau
Copy link

cnassau commented Jul 3, 2015

sage: from sage.sets.set_from_iterator import EnumeratedSetFromIterator
sage: C=CombinatorialFreeModule(ZZ,EnumeratedSetFromIterator(Integers)) ; C
Free module generated by {0, 1, -1, 2, -2, ...} over Integer Ring
sage: U = tensor((C,)) ; U
Free module generated by {0, 1, -1, 2, -2, ...} over Integer Ring
sage: C.basis().category()
Category of enumerated sets
sage: U.basis().category()
Category of finite enumerated sets

But clearly the basis of U is not finite.

Depends on #19195

CC: @tscrim

Component: categories

Author: Christian Nassau

Branch/Commit: u/cnassau/18849 @ 09f97fa

Reviewer: Vincent Delecroix

Issue created by migration from https://trac.sagemath.org/ticket/18849

@cnassau cnassau added this to the sage-6.8 milestone Jul 3, 2015
@cnassau
Copy link
Author

cnassau commented Jul 3, 2015

Author: Christian Nassau

@cnassau
Copy link
Author

cnassau commented Jul 3, 2015

comment:1

Source of this problem is that the tensor product uses a sage.combinat.combinat.MapCombinatorialClass for the cartesian product, and the LazyFamily constructor assumed all CombinatorialClass objects to be finite. An extra check for is_finite has been added in this case. The check is caught since is_finite might not be implemented.

Note that it's important not to treat the negative answer of is_finite as affirmation that the class is actually infinite; whether the set is finite or not might just not be known, or to expansive to check.


New commits:

b553884fix category detection for LazyFamily objects

@cnassau
Copy link
Author

cnassau commented Jul 3, 2015

Branch: u/cnassau/18849

@cnassau
Copy link
Author

cnassau commented Jul 3, 2015

Commit: b553884

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 3, 2015

Changed commit from b553884 to 82849a0

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 3, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

82849a0fix category detection for LazyFamily objects

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 3, 2015

Changed commit from 82849a0 to 09f97fa

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 3, 2015

Branch pushed to git repo; I updated commit sha1. New commits:

09f97fafix category detection for LazyFamily objects

@cnassau
Copy link
Author

cnassau commented Jul 3, 2015

comment:4

A first patchbot run gave this error:

**********************************************************************
File "src/sage/interfaces/expect.py", line 825, in sage.interfaces.expect.Expect._eval_line
Failed example:
    singular.interrupt(timeout=3)  # sometimes very slow (up to 60s on sage.math, 2012)
Expected:
    False
Got:
    True
**********************************************************************
1 item had failures:
   1 of  15 in sage.interfaces.expect.Expect._eval_line
    [89 tests, 1 failure, 9.38 s]
----------------------------------------------------------------------
sage -t --long --warn-long 69.5 src/sage/interfaces/expect.py  # 1 doctest failed
----------------------------------------------------------------------

I think this is completely unrelated and random; I have cleaned up an empty line in the commit, so a new run should be triggered soon.

@videlec
Copy link
Contributor

videlec commented Sep 12, 2015

comment:5

Hello,

  1. Why a special care for CombinatorialClass is even needed at all? The following would be much simpler
category = EnumeratedSets()
if set in Sets().Finite() or isinstance(set, (list,tuple)):
    category = category.Finite()
elif set in Sets().Infinite():
    category = category.Infinite()
  1. Your documentation lines are too long.

Vincent

@videlec
Copy link
Contributor

videlec commented Sep 12, 2015

Reviewer: Vincent Delecroix

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 13, 2021

comment:7

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 19, 2021

comment:8

Setting a new milestone for this ticket based on a cursory review.

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Apr 7, 2022
@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 10, 2022

comment:11

The bug illustrated by the example in the ticket description is still present in 9.7.beta8

The underlying implementation has changed. CombinatorialClass is no longer involved. The bug is now in sage.misc.mrange._is_finite (used by sage.combinat.cartesian_product):

sage: from sage.misc.mrange import _is_finite
sage: from sage.sets.set_from_iterator import EnumeratedSetFromIterator
sage: EnumeratedSetFromIterator(ZZ).category()
Category of facade enumerated sets
sage: _is_finite(EnumeratedSetFromIterator(ZZ))
True

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 10, 2022

comment:12

A comment in _is_finite: "We usually assume L is finite for speed reasons"

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 10, 2022

comment:13

This can be solved by #19195: getting rid of the ancient class sage.combinat.cartesian_product

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 10, 2022

Dependencies: #19195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants