Opened 4 years ago

Closed 3 years ago

#9891 closed defect (fixed)

substitute goes too far: (5-e^x).substitute(x=log(x)) -> 5-log(x)

Reported by: burcin Owned by: burcin
Priority: critical Milestone: sage-4.7.1
Component: symbolics Keywords: pynac
Cc: rbk Merged in: sage-4.7.1.alpha1
Authors: Richard Kreckel Reviewers: Karl-Dieter Crisman
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: Commit:
Dependencies: #11317 Stopgaps:

Description

Reported by Kees on sage-support:

http://groups.google.com/group/sage-support/t/bfa34b077dd31b73

Running the following few lines:

eq=5-e^x
print "1:",eq.substitute(x=3*x)
print "2:",eq.substitute(x=log(x))

yields the output (Sage 4.5.2 with Ubuntu):

1: -e^(3*x) + 5
2: -log(x) + 5

This is also present 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.
> subs(5-exp(x),x==log(x));
5-log(x)

Attachments (1)

trac_9891-substitute.patch (654 bytes) - added by burcin 4 years ago.
add doctest

Download all attachments as: .zip

Change History (11)

comment:1 Changed 4 years ago by rbk

  • Cc kreckel@… added
  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. Developers acknowledge bug.

comment:2 Changed 4 years ago by burcin

  • Authors set to Richard Kreckel
  • Cc rbk added; kreckel@… removed
  • Report Upstream changed from Reported upstream. Developers acknowledge bug. to Fixed upstream, in a later stable release.
  • Status changed from new to needs_work

This was fixed in GiNaC by Richard Kreckel:

http://www.ginac.de/pipermail/ginac-list/2010-September/001738.html

I merged his changes to pynac, so it will be available in the next release.

Changed 4 years ago by burcin

add doctest

comment:3 Changed 3 years ago by kcrisman

Burcin, I'm still getting

----------------------------------------------------------------------
| Sage Version 4.6.2.alpha4, Release Date: 2011-02-07                |
| Type notebook() for the GUI, and license() for information.        |
----------------------------------------------------------------------
**********************************************************************
*                                                                    *
* Warning: this is a prerelease version, and it may be unstable.     *
*                                                                    *
**********************************************************************
sage: (5-e^x).substitute(x=log(x))
-log(x) + 5

Which version of Pynac is this in?

comment:5 Changed 3 years ago by kcrisman

I can confirm this is not in the latest version of Pynac in Sage or on its website (0.2.1), by looking at container.h.

comment:6 Changed 3 years ago by burcin

  • Dependencies set to 11317
  • Status changed from needs_work to needs_review

New pynac package with the fix is at #11317.

comment:7 follow-up: Changed 3 years ago by kcrisman

  • Reviewers set to Karl-Dieter Crisman
  • Status changed from needs_review to positive_review

Okay, this is fine.

Burcin, I have to say I feel a little odd saying that I'm reviewing Kreckel's work, when I'm really reviewing your merging his work and your doctest (though I looked at the actual patch, naturally). Is there a different way to put the author in? You are the author of the actual patch here, not the Ginac update; I don't think any other spkgs put upstream people who committed things in this way.

comment:8 in reply to: ↑ 7 Changed 3 years ago by burcin

Thanks for the review!

Replying to kcrisman:

Burcin, I have to say I feel a little odd saying that I'm reviewing Kreckel's work, when I'm really reviewing your merging his work and your doctest (though I looked at the actual patch, naturally). Is there a different way to put the author in? You are the author of the actual patch here, not the Ginac update; I don't think any other spkgs put upstream people who committed things in this way.

I put Richard's name in the authors list since he should get the credit for fixing this in the Sage release notes. I think we should give more credit to upstream developers. Are you suggesting my name should be in the authors or the reviewers field? :)

comment:9 Changed 3 years ago by jdemeyer

  • Dependencies changed from 11317 to #11317
  • Milestone changed from sage-4.7 to sage-4.7.1

comment:10 Changed 3 years ago by jdemeyer

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