Opened 7 years ago

Closed 7 years ago

#13671 closed defect (fixed)

p.lift(...) results are non-deterministic and incoherent for multivariate polynomials

Reported by: Bouillaguet Owned by: malb
Priority: major Milestone: sage-5.5
Component: commutative algebra Keywords: polynomial rings, mathematically wrong result
Cc: mstreng, malb Merged in: sage-5.5.beta2
Authors: Charles Bouillaguet Reviewers: Marco Streng
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by mstreng)

Merge together with #13675, circular dependency

sage: R.<x1,x2> = QQ[]
sage: I = R.ideal(x2**2 + x1 - 2, x1**2 - 1)
sage: test = I.gen(0) + x2*I.gen(1)
sage: test.lift(I) # correct
[1, x2]
sage: (test+1).lift(I) # invalid input, should give error
[0, 0]
sage: test.lift(I) # incorrect
[0, 0]

p.lift(I) should raise an exception "ValueError: polynomial is not in the ideal" instead of returning [0,0,...,0] in the first place...

Merge together with #13675, circular dependency

Attachments (1)

13671_lift_bug.patch (2.5 KB) - added by Bouillaguet 7 years ago.
Apparently fixing the bug

Download all attachments as: .zip

Change History (12)

comment:1 Changed 7 years ago by malb

Hans (of Singular) wrote:

An additional comment: WerrorS also sets the global variable errorreported, which is used at many places to abort after an error (and to avoid general error message if there was already one (hopefully more specific)). errorreported is reset to 0 in the error handling branch of the singular interpreter. Maybe something equivalent is missing for the routine from libsingular not called via the singular interpreter?

This could be it, we don't reset it.

comment:2 Changed 7 years ago by Bouillaguet

  • Authors set to Charles Bouillaguet
  • Status changed from new to needs_review

comment:3 Changed 7 years ago by mstreng

  • Status changed from needs_review to needs_work

Did you ask Gauss to write the patch for you?

# HG changeset patch
# User Carl Friedrich Gauss <cfgauss@uni-goettingen.de>

You should update the .hmrc file in your home directory (use your own name and email address) and export a new patch here and at #13675.

comment:4 follow-up: Changed 7 years ago by mstreng

ps. Would it be possible to add a check which error was reported by Singular? Just in case self is in I, but Singular reports an error for some other reason (in which case I guess RuntimeError is appropriate).

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

  • Status changed from needs_work to needs_review

Replying to mstreng:

ps. Would it be possible to add a check which error was reported by Singular? Just in case self is in I, but Singular reports an error for some other reason (in which case I guess RuntimeError is appropriate).

I renamed myself and tested that Singular returns 1 when the polynomial is not in the ideal...

comment:6 follow-up: Changed 7 years ago by mstreng

4088	            errorreported = 0 
4089	            if errorreported == 1: 

This looks like it does not work.

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

Replying to mstreng:

4088	            errorreported = 0 
4089	            if errorreported == 1: 

This looks like it does not work.

Good lord, that was stupid.

Changed 7 years ago by Bouillaguet

Apparently fixing the bug

comment:8 follow-up: Changed 7 years ago by mstreng

  • Description modified (diff)
  • Reviewers set to Marco Streng
  • Status changed from needs_review to positive_review

Looks good!

comment:9 in reply to: ↑ 8 ; follow-up: Changed 7 years ago by Bouillaguet

Replying to mstreng:

Looks good!

I don't understand the circular dependency... Why would this one depend on #13675 ?

comment:10 in reply to: ↑ 9 Changed 7 years ago by mstreng

Replying to Bouillaguet:

I don't understand the circular dependency... Why would this one depend on #13675 ?

Not all doctests pass when only this patch is applied.

comment:11 Changed 7 years ago by jdemeyer

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