Ticket #4633 (new defect)

Opened 1 month ago

Last modified 1 month ago

[with patch, needs work] fix additional "Fortran-style" names and a coercion (depends on #4036)

Reported by: bpage Assigned to: was
Priority: major Milestone: sage-3.4
Component: interfaces Keywords: axiom
Cc:

Description

In order to run the the comparison of integration results between FriCAS and Maxima, it is also necessary to make some simple additions to the 'axiom.py' interface: 

wspage@debian:~/sage-3.1.4/devel/sage-main/sage/interfaces$ hg diff
diff -r ed3f78f99d2a sage/interfaces/axiom.py
--- a/sage/interfaces/axiom.py  Tue Nov 25 23:45:43 2008 -0500
+++ b/sage/interfaces/axiom.py  Wed Nov 26 19:43:59 2008 -0500
@@ -729,7 +729,10 @@
        s = P.eval('unparse(%s::InputForm)'%self._name)
        if 'translation error' in s or 'Cannot convert' in s:
            raise NotImplementedError
-        s = multiple_replace({'\r\n':'', # fix stupid Fortran-ish
+        s = multiple_replace({'\r\n':'', # fix stupid Fortran-ish
+                              'DLOG(':'log(',
+                              'DEXP(':'exp(',
+                              '::(':'', ',Symbol)':'',
                              'DSIN(':'sin(',
                              'DCOS(':'cos(',
                              'DTAN(':'tan(',

Integration produce some additional "Fortran-style" names and a coercion that have to be translated before the input form can be processed by Sage.

With this change we can do: 

  test_int = integrand.integrate(x)
  fricas_int = axiom.integrate(integrand,x).sage()
  fricas_cmp = (test_int.simplify_full()-fricas_int.simplify_full()).simplify_full()
  if (fricas_cmp == 0):
      print "FriCAS agrees with Maxima."

Attachments

fortanish.patch (0.8 kB) - added by bpage on 11/26/2008 05:46:11 PM.
patch file
trac_4633.patch (1.0 kB) - added by bpage on 11/26/2008 09:05:01 PM.
[with patch, needs review] Better?

Change History

11/26/2008 05:44:05 PM changed by mabshoff

Hi Bill,

please attach a proper hg patch.

Cheers,

Michael

11/26/2008 05:46:11 PM changed by bpage

  • attachment fortanish.patch added.

patch file

(follow-up: ↓ 3 ) 11/26/2008 05:48:00 PM changed by bpage

Sorry. What is a "hg patch"?

(in reply to: ↑ 2 ; follow-up: ↓ 5 ) 11/26/2008 05:49:25 PM changed by mabshoff

Replying to bpage:

Sorry. What is a "hg patch"?

A mercurial patch. hg is the chemical symbol for mercurial, so that is why the mercurial binary is called hg.

Cheers,

Michael

11/26/2008 05:50:28 PM changed by bpage

Further note: The new release of FriCAS-1.0.4 which is not yet available as a Sage spkg no longer produces this "Fortranish" functions names.

(in reply to: ↑ 3 ) 11/26/2008 05:52:58 PM changed by bpage

A mercurial patch. hg is the chemical symbol for mercurial, so that is why the mercurial binary is called hg.

:~) I meant, how do I create a "hg patch" if not by "hg diff"?

11/26/2008 06:01:53 PM changed by mabshoff

Hi Bill,

  • Check in your changes: hg commit
  • export the last commit: hg export tip > trac_4633.patch
  • attach patch to this ticket
  • prepend "[with patch, needs review] to summary line
  • wait for review

Cheers,

Michael

11/26/2008 09:05:01 PM changed by bpage

  • attachment trac_4633.patch added.

[with patch, needs review] Better?

(follow-up: ↓ 8 ) 11/30/2008 01:42:53 PM changed by tjlahey

  • summary changed from fix additional "Fortran-style" names and a coercion to [with patch, needs review] fix additional "Fortran-style" names and a coercion.

Is there a specific procedure for installing (and testing) these kinds of patches? That is, how do I get Sage to know that the code has been updated? This kind of thing would be nice on a wiki page for new developers. Or have I just missed it?

(in reply to: ↑ 7 ; follow-up: ↓ 9 ) 11/30/2008 07:54:05 PM changed by bpage

Replying to tjlahey:

Is there a specific procedure for installing (and testing) these kinds of patches? That is, how do I get Sage to know that the code has been updated? This kind of thing would be nice on a wiki page for new developers. Or have I just missed it?

This is what I do:

  1. Pretest, e.g. try: axiom(1/log(x)).sage() 
        NotImplementedError
  2. Click on the link to that patch: "trac_4633.patch"
  3. Click on the link to "original format"
  4. Save the patch file somewhere, e.g. ~/trac_4633.patch
  5. cd sage*/devel/sage-main
  6. apply the patch: patch -p1 < ~/trac_4633.patch
  7. re-build sage: sage -br
  8. test, e.g. try: axiom(1/log(x)).sage() 
        1/log(x)

I guess that in a more complete patch I should have added some tests like this to the doc tests. This just seemed too simple.

(in reply to: ↑ 8 ) 12/02/2008 03:23:04 PM changed by tjlahey

Replying to bpage:

It doesn't want to apply. In my version of sage/interfaces/axiom.py, I don't have: 

         s = P.eval('unparse(%s::InputForm)'%self._name) 
         if 'translation error' in s or 'Cannot convert' in s: 
             raise NotImplementedError

so I get a rejection when I attempt to apply the patch. I'm using Sage 3.2.

(follow-up: ↓ 11 ) 12/02/2008 03:24:44 PM changed by mabshoff

This is probably on top of #4036, which itself needs to be slightly rebased.

Cheers,

Michael

(in reply to: ↑ 10 ) 12/02/2008 04:20:09 PM changed by bpage

Replying to mabshoff:

This is probably on top of #4036, which itself needs to be slightly rebased.

Correct. What is the proper procedure to follow in this case?

12/02/2008 04:23:15 PM changed by mabshoff

  • summary changed from [with patch, needs review] fix additional "Fortran-style" names and a coercion to [with patch, needs review] fix additional "Fortran-style" names and a coercion (depends on #4036).

Hi Bill,

Just mention it on the ticket and/or change the summary line like I just did.

Cheers,

Michael

12/06/2008 02:15:03 PM changed by was

  • summary changed from [with patch, needs review] fix additional "Fortran-style" names and a coercion (depends on #4036) to [with patch, needs work] fix additional "Fortran-style" names and a coercion (depends on #4036).

Please add a doctest that illustrates what you're fixing and mark it "#optional - fricas". The doctest could also include your integration example.

Thanks!!