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

Bug in conjugate of symbolic ring #8775

Closed
kcrisman opened this issue Apr 27, 2010 · 20 comments
Closed

Bug in conjugate of symbolic ring #8775

kcrisman opened this issue Apr 27, 2010 · 20 comments

Comments

@kcrisman
Copy link
Member

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.

Component: symbolics

Keywords: pynac

Author: Burcin Erocal

Reviewer: Luis Felipe Tabera Alonso

Merged: sage-4.6.1.alpha1

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

@kcrisman
Copy link
Member Author

Upstream: Not yet reported upstream; Will do shortly.

@kcrisman
Copy link
Member Author

comment:1

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".

@kcrisman
Copy link
Member Author

comment:2

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

@kcrisman
Copy link
Member Author

Changed upstream from Not yet reported upstream; Will do shortly. to Reported upstream. Little or no feedback.

@burcin
Copy link

burcin commented May 6, 2010

add doctests

@burcin
Copy link

burcin commented May 6, 2010

Author: Burcin Erocal

@burcin
Copy link

burcin commented May 6, 2010

Changed keywords from none to pynac

@burcin
Copy link

burcin commented May 6, 2010

comment:3

Attachment: trac_8775-conjugate.patch.gz

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.

@kcrisman
Copy link
Member Author

comment:4

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.

@burcin
Copy link

burcin commented Sep 12, 2010

comment:5

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.

@burcin
Copy link

burcin commented Sep 12, 2010

apply only this patch

@burcin
Copy link

burcin commented Sep 12, 2010

comment:6

Attachment: trac_8775-conjugate.take2.patch.gz

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.

@burcin
Copy link

burcin commented Sep 12, 2010

Changed upstream from Reported upstream. Little or no feedback. to none

@lftabera
Copy link
Contributor

lftabera commented Nov 6, 2010

comment:7

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

@lftabera
Copy link
Contributor

lftabera commented Nov 6, 2010

Reviewer: Luis Felipe Tabera

@jdemeyer
Copy link

jdemeyer commented Nov 7, 2010

comment:8

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

@jdemeyer
Copy link

Same patch with fixed commit message

@jdemeyer
Copy link

Merged: sage-4.6.1.alpha1

@jdemeyer
Copy link

comment:9

Attachment: trac_8775-conjugate-fixed-message.patch.gz

@jdemeyer
Copy link

Changed reviewer from Luis Felipe Tabera to Luis Felipe Tabera Alonso

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

4 participants