Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#8775 closed defect (fixed)

Bug in conjugate of symbolic ring

Reported by: kcrisman Owned by: burcin
Priority: critical Milestone: sage-4.6.1
Component: symbolics Keywords: pynac
Cc: Merged in: sage-4.6.1.alpha1
Authors: Burcin Erocal Reviewers: Luis Felipe Tabera Alonso
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

From http://groups.google.com/group/sage-devel/browse_thread/thread/9f941378a95c0191:

sage: a = sqrt(-3) 
sage: a 
sqrt(-3) 
sage: a.conjugate() 
sqrt(-3) 
sage: bool(a==a.conjugate()) 
True 

Could this be related to #6244? Anyway, presumably conjugate should remain unevaluated on this sort of thing, while still being evaluated on things like a+I or 33.

Attachments (3)

trac_8775-conjugate.patch (1.6 KB) - added by burcin 10 years ago.
add doctests
trac_8775-conjugate.take2.patch (1007 bytes) - added by burcin 10 years ago.
apply only this patch
trac_8775-conjugate-fixed-message.patch (1007 bytes) - added by jdemeyer 10 years ago.
Same patch with fixed commit message

Download all attachments as: .zip

Change History (13)

comment:1 Changed 10 years ago by kcrisman

  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.

From Burcin Erocal on the same thread:

This is a bug in GiNaC: 
ginsh - GiNaC Interactive Shell (ginac V1.5.7) 
  __,  _______  Copyright (C) 1999-2010 Johannes Gutenberg University Mainz, 
 (__) *       | Germany.  This is free software with ABSOLUTELY NO WARRANTY. 
  ._) i N a C | You are welcome to redistribute it under certain conditions. 
<-------------' For details type `warranty;'. 
Type ?? for a list of help topics. 
> sqrt(-3); 
sqrt(-3) 
> conjugate(sqrt(-3)); 

sqrt(-3) 
For conjugation, power objects just compute the conjugate of the basis 
and the exponent, and construct a new power object from these. Here is 
the relevant function: 
http://pynac.sagemath.org/hg/file/3ece9ba22005/ginac/power.cpp#l805 

I'm changing this to "not yet reported upstream".

comment:2 Changed 10 years ago by kcrisman

  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. Little or no feedback.

Changing upstream report - too early for feedback at this point.

Changed 10 years ago by burcin

add doctests

comment:3 Changed 10 years ago by burcin

  • Authors set to Burcin Erocal
  • Keywords pynac added
  • Status changed from new to needs_review

This is fixed by the new pynac package at #8903. attachment:trac_8775-conjugate.patch contains doctest fixes.

Note that the new pynac version also fixes #8542, #8651, and #8688. Patches from these tickets should be applied before running doctests.

comment:4 follow-up: Changed 10 years ago by kcrisman

  • Status changed from needs_review to needs_work

For some reason, although Sage 4.4.4.alpha0 has pynac-0.2.0.p3

----------------------------------------------------------------------
| Sage Version 4.4.4.alpha0, Release Date: 2010-06-07                |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
sage: N(sqrt(-2),200)
8.0751148893563733350506651837615871941533119425962889089783e-62 + 1.4142135623730950488016887242096980785696718753769480731767*I
sage: conjugate(sqrt(-3))
sqrt(-3)

Did this change not end up making it into the Pynac package after all? According to http://pynac.sagemath.org/hg/rev/60acd6985820, it should be in there, but now I find it hard to explain the above.

comment:5 in reply to: ↑ 4 Changed 10 years ago by burcin

Replying to kcrisman:

Did this change not end up making it into the Pynac package after all? According to http://pynac.sagemath.org/hg/rev/60acd6985820, it should be in there, but now I find it hard to explain the above.

That patched was backed out since it caused some problems with doctests in sage/rings/qqbar.py.

I merged the upstream patch from GiNaC fixing this problem in the latest version of pynac. I will upload a new patch with doctest fixes later.

Changed 10 years ago by burcin

apply only this patch

comment:6 Changed 10 years ago by burcin

  • Report Upstream changed from Reported upstream. Little or no feedback. to N/A
  • Status changed from needs_work to needs_review

I uploaded a new patch to add doctests for the fixes in Pynac. Only attachment:trac_8775-conjugate.take2.patch should be applied.

This depends on #9901.

comment:7 Changed 10 years ago by lftabera

  • Reviewers set to Luis Felipe Tabera
  • Status changed from needs_review to positive_review

The issue seems to be solved. I have tried other examples and it works as expected. The doctest passes. Positive review

comment:8 Changed 10 years ago by jdemeyer

  • Status changed from positive_review to needs_work

There is a typo in the ticket number in the commit message :-)

Changed 10 years ago by jdemeyer

Same patch with fixed commit message

comment:9 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha1
  • Resolution set to fixed
  • Status changed from needs_work to closed

comment:10 Changed 10 years ago by jdemeyer

  • Reviewers changed from Luis Felipe Tabera to Luis Felipe Tabera Alonso
Note: See TracTickets for help on using tickets.