Opened 7 years ago

Closed 7 years ago

#13155 closed defect (fixed)

Boolean Multivariate Ideals should not have negative dimension....

Reported by: Bouillaguet Owned by: malb
Priority: minor Milestone: sage-5.2
Component: commutative algebra Keywords: polybori
Cc: malb, AlexanderDreyer, PolyBori Merged in: sage-5.2.beta1
Authors: Charles Bouillaguet Reviewers: Martin Albrecht
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by Bouillaguet)

The dimension of an ideal cannot be negative (it would be mathematically incoherent). Yet, in SAGE, it is possible to create Boolean Ideals of dimension -1.....

sage: n=11
sage: R = BooleanPolynomialRing(n, 'x')
sage: R2 = PolynomialRing(GF(2), n, 'x')
sage: I = ideal([ R(f) for f in sage.rings.ideal.Cyclic(R2, n).gens() ])
sage: I.dimension()
-1

In fact, all the BooleanPolynomialIdeal's should have dimension zero. Thus I suggest to overload the dimension() method to just return zero....

Attachments (1)

pbori_dimension_fix.2.patch (1.0 KB) - added by Bouillaguet 7 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by malb

Agreed!

comment:2 Changed 7 years ago by Bouillaguet

  • Priority changed from major to minor
  • Status changed from new to needs_review

comment:3 Changed 7 years ago by malb

  • Reviewers set to Martin Albrecht
  • Status changed from needs_review to needs_work

Looks good except for one minor detail

TESTS:: 
    # check that Ticket #13155 is solved 

should be rewritten as:

TESTS:

Check that :trac:`13155` is solved::

    sage: ...

So there's an empty line between :: and the Sage tests (which I believe is required) and it uses the :trac:`foo` directive.

comment:4 Changed 7 years ago by Bouillaguet

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:5 follow-up: Changed 7 years ago by malb

Sorry for being picky but there's still an issue there. The :: indicates that the code block follows (which is then used for doctests. You have two of those one after TESTS and one after the sentence. You need to drop the one after TESTS

Changed 7 years ago by Bouillaguet

comment:6 in reply to: ↑ 5 Changed 7 years ago by Bouillaguet

Replying to malb:

Sorry for being picky but there's still an issue there. The :: indicates that the code block follows (which is then used for doctests. You have two of those one after TESTS and one after the sentence. You need to drop the one after TESTS

I will eventually learn to do this right on the first try... Patch updated.

comment:7 Changed 7 years ago by malb

  • Status changed from needs_review to positive_review

all good now

comment:8 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.2.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.