Opened 5 years ago

Closed 5 years ago

#21231 closed enhancement (fixed)

improve FriCAS interface

Reported by: mantepse Owned by:
Priority: major Milestone: sage-7.4
Component: interfaces Keywords: FriCAS
Cc: bpage, hemmecke, dkrenn, dimpase Merged in:
Authors: Martin Rubey Reviewers: Bill Page, Emmanuel Charpentier
Report Upstream: N/A Work issues:
Branch: e281742 (Commits, GitHub, GitLab) Commit: e281742685eb891ff7883bd6e91b16e8c31b92d6
Dependencies: #21209 Stopgaps:

Status badges

Description

The FriCAS interface is currently very rudimentary. In particular, converting the results of a computation into sage types is available only in very few special cases.

Attachments (2)

sage-7.2-fricas.pdf (67.9 KB) - added by bpage 5 years ago.
Sage worksheet under 7.2
sage-7.4.beta0-fricas.pdf (65.8 KB) - added by bpage 5 years ago.
Sage worksheet under 7.4.beta0 (typeset output fails)

Download all attachments as: .zip

Change History (111)

comment:1 Changed 5 years ago by mantepse

1) implement a method that (reliably!) yields a tuple (type, 1d algebra output, 2d algebra output, error message, etc) as strings. 1d algebra output should come in "unparsed inputform" as one very long string, I'd say. 2d algebra output should be empty if 1d output is available.

It should be able to deal with very long lines, too, possibly be using file io. This involves use of ioHook and some regexps.

2) implement a method that translates fricas output into sage types.

I think this could work as follows: we want to map fricas types to sage constructors, possibly recursively. Recall that what's really sent to fricas is something like "sage23 := [x^n/y^n for n in 1..3]", so that sage can access the result using the variable "sage23". This string is referred to as self._name

Now what I propose is a method which takes a parsed type, e.g., "Integer" or ("List", "Integer") or ("UnivariatePolynomial", "x", ("Fraction", "Integer"))

def to_sage(type):
    if type == "Integer":
         return to_sage_integer()
    elif type == "String":
         return to_sage_string()
    ...
    elif isinstance(type, tuple):
        if type[0] == "List":
            return to_sage_list(type[1])
        elif type[0] == "Fraction":
            return to_sage_fraction(type[1])
        ....

and so on.

So, if the type is ("List" ("Fraction" ("UnivariatePolynomial" "x" "Integer"))), this method would call

    to_sage_list(("Fraction" ("UnivariatePolynomial" "x" "Integer"))

which might be something like

def to_sage_list(self, type):
    n = fricas.eval("#" + self._name).to_sage_integer()
    return [fricas.eval(self._name + ".%n").to_sage(type) for n in range(1,n+1)]

comment:2 Changed 5 years ago by mantepse

  • Type changed from PLEASE CHANGE to enhancement

comment:3 Changed 5 years ago by mantepse

  • Branch set to /u/mantepse/21231
  • Commit set to 5b7e6ab305a5d4967bd658e9ce991683a4ebdac9

New commits:

5fa5c04Merge branch 'u/mantepse/generating_function_in_findstat_interface' of git://trac.sagemath.org/sage into develop
4b555c5Merge branch 'develop' of git://github.com/sagemath/sage into develop
17f7dbeMerge branch 'u/dkrenn/16137' of git://trac.sagemath.org/sage into develop
08494a2Merge branch 'u/vdelecroix/16137' of git://trac.sagemath.org/sage into develop
f077211Merge branch 'develop' of git://github.com/sagemath/sage into develop
5b7e6abinitial version of new fricas interface

comment:4 Changed 5 years ago by mantepse

This is very beta, but I'd be very happy to receive some comments.

comment:5 Changed 5 years ago by mantepse

  • Authors set to Martin Rubey
  • Commit changed from 5b7e6ab305a5d4967bd658e9ce991683a4ebdac9 to d8a26e149d4f182817c3664af11c38b8f55488c4

New commits:

d8a26e1Merge remote-tracking branch 'origin/develop' into fricas-interface

comment:6 follow-up: Changed 5 years ago by vbraun

  • Branch changed from /u/mantepse/21231 to u/mantepse/21231
  • Commit changed from d8a26e149d4f182817c3664af11c38b8f55488c4 to b2ee8cca6b2ce9320f31b3c457b86f048dd66a54

Branch name doesn't have leading slash


New commits:

b2ee8ccmake it build

comment:7 in reply to: ↑ 6 ; follow-up: Changed 5 years ago by mantepse

Replying to vbraun:

Branch name doesn't have leading slash

Howto? I used

git push --set-upstream trac HEAD:u/mantepse/21231

comment:8 in reply to: ↑ 7 Changed 5 years ago by dimpase

Replying to mantepse:

Replying to vbraun:

Branch name doesn't have leading slash

Howto? I used

git push --set-upstream trac HEAD:u/mantepse/21231

This is fine, this does not do anything to the webpage contents. You made a typo, this leading slash, while editing the latter.

comment:9 Changed 5 years ago by dimpase

  • Dependencies set to #21209

comment:10 Changed 5 years ago by mantepse

(it actually doesn't depend on #21209, since it works with any installation of FriCAS but never mind)

comment:11 Changed 5 years ago by git

  • Commit changed from b2ee8cca6b2ce9320f31b3c457b86f048dd66a54 to cbcb94f2cf3b54926be73c4eb842df5da0096d73

Branch pushed to git repo; I updated commit sha1. New commits:

cbcb94fquick dirty fix for bug reported by Bill Page, add idea for better type handling

Changed 5 years ago by bpage

Sage worksheet under 7.2

Changed 5 years ago by bpage

Sage worksheet under 7.4.beta0 (typeset output fails)

comment:12 Changed 5 years ago by bpage

In Sage Worksheet typeset output or FriCAS object fails with this patch on 7.4.beta0. See attached pdf files.

https://trac.sagemath.org/attachment/ticket/21231/sage-7.2-fricas.pdf (typeset output OK)

https://trac.sagemath.org/attachment/ticket/21231/sage-7.4.beta0-fricas.pdf (typeset output fails)

I am not sure whether or not this example worked under 7.4.beta0 without the patch.

comment:13 Changed 5 years ago by git

  • Commit changed from cbcb94f2cf3b54926be73c4eb842df5da0096d73 to 5b4a95d69c1e43fd29f18a07a2d1b7bb6f904459

Branch pushed to git repo; I updated commit sha1. New commits:

5b4a95dswitch to better type treatment

comment:14 Changed 5 years ago by git

  • Commit changed from 5b4a95d69c1e43fd29f18a07a2d1b7bb6f904459 to 0ab0f924f297669e203031ab9f74677eee7c8361

Branch pushed to git repo; I updated commit sha1. New commits:

0ab0f92further improve translation to sage objects, make restart work, fix doctests

comment:15 Changed 5 years ago by mantepse

I have a question about the _fricas_init_ and _fricas_ methods defined in various files, in particular in ~/sage-develop/src/sage/rings/rational_field.py and `in ~/sage-develop/src/sage/rings/real_mpfr.pyx`, see below.

The question is: the doctests work without adapting the methods. What are the methods supposed to do?

~/sage-develop/src/sage/rings/rational_field.py

    def _axiom_init_(self):
        r"""
        Return the axiom/fricas representation of `\QQ`.

        EXAMPLES::

           sage: axiom(QQ)    #optional - axiom # indirect doctest
           Fraction Integer
           sage: fricas(QQ)   #optional - fricas # indirect doctest
           Fraction(Integer)

        """
        return 'Fraction Integer'

    _fricas_init_ = _axiom_init_

in ~/sage-develop/src/sage/rings/real_mpfr.pyx

    def _axiom_(self, axiom):
        """
        Return ``self`` as a floating point number in Axiom.

        EXAMPLES::

            sage: R = RealField(100)
            sage: R(pi)
            3.1415926535897932384626433833
            sage: axiom(R(pi))  # optional - axiom # indirect doctest
            3.1415926535 8979323846 26433833
            sage: fricas(R(pi)) # optional - fricas
            3.1415926535_8979323846_26433833

        """
        prec = self.parent().prec()

        #Set the precision in Axiom
        old_prec = axiom('precision(%s)$Float'%prec)
        res = axiom('%s :: Float'%self.exact_rational())
        axiom.eval('precision(%s)$Float'%old_prec)

        return res

    _fricas_ = _axiom_

comment:16 Changed 5 years ago by git

  • Commit changed from 0ab0f924f297669e203031ab9f74677eee7c8361 to d2d0fb89696d255c0a583815832b2302e63d1215

Branch pushed to git repo; I updated commit sha1. New commits:

d2d0fb8fix tab completion

comment:17 Changed 5 years ago by mantepse

The once missing thing is the error handling. I was expecting that raising an error in eval would be sufficient, as indicated by the patch below. However, if I then do fricas("something stupid"), no error is raised. What am I doing wrong?

--- a/src/sage/interfaces/fricas.py
+++ b/src/sage/interfaces/fricas.py
@@ -385,8 +385,7 @@ class FriCAS(ExtraTabCompletion, Expect):
                 return "\r\n".join(line[FRICAS_MULTI_LINE_START:] for line in lines)
 
         else:
-            print(output)
-            return
+            raise RuntimeError("FriCAS has not recognized '%s' as a valid operation: %s" % (code, output))
 
     def set(self, var, value):
         """Set the variable var to the given value.

comment:18 Changed 5 years ago by bpage

Calling fricas("something stupid") ends up in fricas.set which calls expect._eval_line not fricas.eval. This interface stuff is truly twisted. Check especially the inheritance. FriCAS inherits from Expect that inherits Interface. Check __call__ and _create in Interface and go from there.

fricas.py should probably override _eval_line in the same way as axiom.py.

Last edited 5 years ago by bpage (previous) (diff)

comment:19 Changed 5 years ago by git

  • Commit changed from d2d0fb89696d255c0a583815832b2302e63d1215 to 148bf77e2d3504f9d3dbbda038ca4270dd81bd83

Branch pushed to git repo; I updated commit sha1. New commits:

148bf77better to ask for forgiveness than permission in __getitem__, some series examples, raise error in eval

comment:20 follow-up: Changed 5 years ago by bpage

fricas.py used to inherit some functions from axiom.py. The new version no longer uses any part of the Axiom interface. One of the casualties of this change was _latex_ from PanAxiomElement. The function _latex_ is responsible for producing LaTeX output in typeset mode. The following patch incorporates the _latex_ function from axiom.py and corrects the problem reported above in https://trac.sagemath.org/ticket/21231#comment:12

diff --git a/src/sage/interfaces/fricas.py b/src/sage/interfaces/fricas.py
index 8831c6c..5c1bc64 100644
--- a/src/sage/interfaces/fricas.py
+++ b/src/sage/interfaces/fricas.py
@@ -201,6 +201,7 @@ from __future__ import print_function
 
 from sage.interfaces.tab_completion import ExtraTabCompletion
 from sage.interfaces.expect import Expect, ExpectElement, FunctionElement, ExpectFunction
+from sage.misc.multireplace import multiple_replace
 from sage.env import DOT_SAGE
 import re
 import six
@@ -571,6 +572,31 @@ class FriCASElement(ExpectElement):
             raise IndexError("index out of range")
         return self.elt(n+1)
 
+    def _latex_(self):
+        r"""
+        EXAMPLES::
+
+            sage: a = fricas(1/2) #optional - fricas
+            sage: latex(a)       #optional - fricas
+             1 \over 2
+
+        """
+        self._check_valid()
+        P = self.parent()
+        s = P._eval_line('outputAsTex(%s)'%self.name())
+        if not '$$' in s:
+            raise RuntimeError("Error texing axiom object.")
+        i = s.find('$$')
+        j = s.rfind('$$')
+        s = s[i+2:j]
+        s = multiple_replace({'\r':'', '\n':' ',
+                              ' \\sp ':'^',
+                              '\\arcsin ':'\\sin^{-1} ',
+                              '\\arccos ':'\\cos^{-1} ',
+                              '\\arctan ':'\\tan^{-1} '},
+            re.sub(r'\\leqno\(.*?\)','',s)) # no eq number!
+        return s
+
     def __int__(self):
         return int(self.sage())

I am not sure of the relevance of the particular substitutions that were performed by the old axiom.py code.

Sorry for just including a patch but I am not sure whether or not I could have just pushed this change to trac since this branch seems to be private.

comment:21 Changed 5 years ago by bpage

FriCAS uses

sage: fricas('asin(x)')
asin(x)
sage: fricas('acos(x)')
acos(x)
sage: fricas('atan(x)')
atan(x)

Sage replaces asin, acos and atan with arcsin, arccos and arctan

sage: asin(x)
arcsin(x)
sage: acos(x)
arccos(x)
sage: atan(x)
arctan(x)

But FriCAS does not use these names so the following results in an error:

sage: fricas(asin(x))
...
   There are no library operations named arcsin 
      Use HyperDoc Browse or issue
                                                                                                                   )what op arctan
      to learn if there is any operation containing " arctan " in its name.
 ...
sage: fricas(acos(x))
...
   There are no library operations named arccos 
      Use HyperDoc Browse or issue
                                                                                                                   )what op arctan
      to learn if there is any operation containing " arctan " in its name.
 ...
sage: fricas(atan(x))
...
   There are no library operations named arctan 
      Use HyperDoc Browse or issue
                                                                                                                   )what op arctan
      to learn if there is any operation containing " arctan " in its name.
 ...

comment:22 in reply to: ↑ 20 ; follow-up: Changed 5 years ago by mantepse

Sorry for just including a patch but I am not sure whether or not I could have just pushed this change to trac since this branch seems to be private.

Putting patches here is perfect for me, actually! Many thanks!

comment:23 in reply to: ↑ 22 Changed 5 years ago by dimpase

Replying to mantepse:

Sorry for just including a patch but I am not sure whether or not I could have just pushed this change to trac since this branch seems to be private.

Putting patches here is perfect for me, actually! Many thanks!

although you could have just pushed your own (i.e. u/bpage) or public (i.e. public/) branch; then your commit(s) could be cherry-picked. An advantage is that it's no error-prone cutting/pasting, (and it is known whom to blame :-)).

comment:24 Changed 5 years ago by git

  • Commit changed from 148bf77e2d3504f9d3dbbda038ca4270dd81bd83 to ca3277c6413ee5fb08e43f2ee48cf26fec5ec294

Branch pushed to git repo; I updated commit sha1. New commits:

ca3277cfix eval

comment:25 Changed 5 years ago by mantepse

  • Status changed from new to needs_review

comment:26 Changed 5 years ago by bpage

Typeset output from FriCAS in the notebook is not working. Apparently the most recent commit is still missing _latex_.

comment:27 Changed 5 years ago by bpage

I think the markers |startKeyedMsg| and |endOfKeyedMsg| look strange in the error message and traceback. In the notebook only |endOfKeyedMsg| is shown as the output until clicking to expand the traceback. These markers should probably be removed from the output before displaying the message to the user.

comment:28 Changed 5 years ago by mantepse

Thanks for reminding me of LaTeX, I'm embarassed - I forgot!

Removing |startKeyedMsg| and |endOfKeyedMsg| and the like is not completely trivial, but I'll try!

comment:29 Changed 5 years ago by mantepse

  • Status changed from needs_review to needs_work

comment:30 Changed 5 years ago by git

  • Commit changed from ca3277c6413ee5fb08e43f2ee48cf26fec5ec294 to b5f5a1c8ca4699a8d55cfa37ed9813b89f84ac5d

Branch pushed to git repo; I updated commit sha1. New commits:

b5f5a1cfix latex, translate Factored, conversions for expressions

comment:31 follow-up: Changed 5 years ago by bpage

Thanks, Martin. Here is a small patch that fixes a few problems with FriCAS-generated LaTeX. For example:

sage: latex(fricas("integrate(sin(x+1/x),x)"))
 \int ^{\displaystyle x} {{\sin  \left( {{{{{ \%A} ^{2}}+1} \over \%A}}  \right)} \  {d \%A}}   

and also subscripts and subscripts in a few other cases.

diff --git a/src/sage/interfaces/fricas.py b/src/sage/interfaces/fricas.py
index 8d2ac3f..594d5b8 100644
--- a/src/sage/interfaces/fricas.py
+++ b/src/sage/interfaces/fricas.py
@@ -598,7 +598,10 @@ class FriCASElement(ExpectElement):
         j = s.rfind('$$')
         s = s[i+2:j]
         s = multiple_replace({'\r':'', '\n':' ',
-                              ' \\sp ':'^',
+                              '\\sp ':'^',
+                              '\\sp{':'^{',
+                              '\\sb ':'_',
+                              '\\sb{':'_{',
                               '\\arcsin ':'\\sin^{-1} ',
                               '\\arccos ':'\\cos^{-1} ',
                               '\\arctan ':'\\tan^{-1} '},

comment:32 Changed 5 years ago by git

  • Commit changed from b5f5a1c8ca4699a8d55cfa37ed9813b89f84ac5d to ccb26de5806aa228e66ca44b47e5611dcf41b58e

Branch pushed to git repo; I updated commit sha1. New commits:

ccb26deMerge branch 'develop' of git://trac.sagemath.org/sage into fricas-interface

comment:33 Changed 5 years ago by mantepse

  • Status changed from needs_work to needs_review

comment:34 follow-up: Changed 5 years ago by mantepse

Hi Bill,

I don't understand your patch: why would you replace \sp{ by ^{ if you replace \sp by ^ anyway. Also, isn't \sp valid LaTeX?

But more importantly: shouldn't this patch really applied to FriCAS itself?

comment:35 Changed 5 years ago by hemmecke

Maybe tex.spad from my mathjax branch might be interesting. It spits out true latex. https://github.com/hemmecke/fricas/commits/mathjax But maybe you really want mathjax or even 1d output of FriCAS objects, then look into mathjax.spad or 1d.spad. Most importantly, the output form can be changed at runtime.

Note that these files are GPL3+ and are therefore not (yet) part of FriCAS.

comment:36 follow-up: Changed 5 years ago by mantepse

Hi Bill and Ralf!

I'd like to get this ticket in soon (and in particular, stop working on it very soon). So if FriCAS produces only TeX currently, I'd like to leave it at this. As soon as the new tex.spad is in FriCAS, we can pick it up here, OK?

comment:37 in reply to: ↑ 31 ; follow-up: Changed 5 years ago by mantepse

                               '\\arcsin ':'\\sin^{-1} ',
                               '\\arccos ':'\\cos^{-1} ',
                               '\\arctan ':'\\tan^{-1} '},

another question: why are you replacing \arctan and friends? As far as I know, this is perfectyl valid LaTeX, no?

comment:38 Changed 5 years ago by hemmecke

Yes, it is. And given the usual meaning of sin2(x) = (sin(x))2 and not sin(sin(x)), I even think that writing sin(-1) for arcsin is even confusing, no?

comment:39 Changed 5 years ago by git

  • Commit changed from ccb26de5806aa228e66ca44b47e5611dcf41b58e to 8871cb226009084007a60c30dbe62e4a28e83fc8

Branch pushed to git repo; I updated commit sha1. New commits:

8871cb2tiny latex fix

comment:40 Changed 5 years ago by git

  • Commit changed from 8871cb226009084007a60c30dbe62e4a28e83fc8 to 2fe8ea595ef1f63ccc4d67cefe5688d314b95cdf

Branch pushed to git repo; I updated commit sha1. New commits:

2fe8ea5forgot to raise NotImplementedError

comment:41 in reply to: ↑ 34 Changed 5 years ago by bpage

Replying to mantepse:

Hi Bill,

I don't understand your patch: why would you replace \sp{ by ^{ if you replace \sp by ^ anyway.

Note the space '\sp '. It is looking for termination of \sp.

Also, isn't \sp valid LaTeX?

Not as far as I know - maybe TeX? In any case it does not work in Sage notebook.

But more importantly: shouldn't this patch really applied to FriCAS itself?

Perhaps. Waldek would probably accept a patch to that effect.

comment:42 in reply to: ↑ 36 Changed 5 years ago by bpage

Replying to mantepse:

Hi Bill and Ralf!

I'd like to get this ticket in soon (and in particular, stop working on it very soon). So if FriCAS produces only TeX currently, I'd like to leave it at this. As soon as the new tex.spad is in FriCAS, we can pick it up here, OK?

I think Waldek is unlikely to change his position on the exclusion of copyleft licensed code in FriCAS since FriCAS already has a more permissive license. Of course that does not prevent its inclusion in the FriCAS external package here if someone were so inclined.

comment:43 in reply to: ↑ 37 Changed 5 years ago by bpage

Replying to mantepse:

another question: why are you replacing \arctan and friends? As far as I know, this is perfectyl valid LaTeX, no?

Yes. I agree with this change. I just copied (with some reservations) what was added to the Axiom interface.

comment:44 Changed 5 years ago by bpage

Error output in Sage notebook still looks like this

fricas("something stupid")

Traceback (click to the left of this block for traceback)
...
|endOfKeyedMsg|

Expanded:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
...
  File "/home/wspage/sage/local/lib/python2.7/site-packages/sage/interfaces/fricas.py", line 399, in set
    raise RuntimeError("An error occurred when FriCAS evaluated '%s': %s" % (value, output))
TypeError: An error occurred when FriCAS evaluated 'something stupid': |startKeyedMsg|
   There are no library operations named something 
      Use HyperDoc Browse or issue
                               )what op something
      to learn if there is any operation containing " something " in its 
      name.
|endOfKeyedMsg|
 
|startKeyedMsg|
   Cannot find a definition or applicable library operation named 
      something with argument type(s) 
                                Variable(stupid)
      
      Perhaps you should use "@" to indicate the required return type, or 
      "$" to specify which version of the function you need.
|endOfKeyedMsg|

It would be nice to get rid of |startKeyedMsg| and |endOfKeyedMsg| and include something more meaningful in the unexpanded version.

comment:45 Changed 5 years ago by bpage

Conversion of the following FriCAS result to Sage fails.

sage: fricas("integrate(sin(x+1/x),x)")

   x       2
 ++      %A  + 1
 |   sin(-------)d%A
++          %A

sage: fricas("integrate(sin(x+1/x),x)").sage()
...
TypeError: Bad function call: integral(sin((x^2+1)/x),x: !!! :Symbol)

comment:46 Changed 5 years ago by git

  • Commit changed from 2fe8ea595ef1f63ccc4d67cefe5688d314b95cdf to 41aefd7bcd18edbbee79a88ba8321e8f689f2fa0

Branch pushed to git repo; I updated commit sha1. New commits:

43c1136Exclude plain text from IPython super call
b027498Merge branch 'u/vbraun/call_repr_once' of git://trac.sagemath.org/sage into develop
2006541Merge branch 'u/mantepse/21231' of git://trac.sagemath.org/sage into develop
41aefd7incorporate reviewers suggestions, in particular better error handling

comment:47 Changed 5 years ago by bpage

Excellent, thanks! This now looks great to me.

comment:48 Changed 5 years ago by bpage

  • Reviewers set to bpage
  • Status changed from needs_review to positive_review

comment:49 Changed 5 years ago by bpage

  • Status changed from positive_review to needs_work

Oops, I was a bit too hasty and enthusiastic. Just one last thing ...

Try this in the sage --notebook

%fricas
x+1

I see the following markers displayed

|startAlgebraOutput|
   x + 1
|endOfAlgebraOutput|

Can you get rid of the markers in this case too?

comment:50 Changed 5 years ago by mantepse

if

%fricas
x+1

displays the output of fricas.eval(1+1) instead of fricas(1+1), this is going to be rather hard. How could I find out?

Last edited 5 years ago by mantepse (previous) (diff)

comment:51 Changed 5 years ago by bpage

fricas.eval calls _eval_line inherited from Expect. Maybe you should just override it?

comment:52 Changed 5 years ago by mantepse

I wanted to avoid overriding eval and _eval_line because they do not have much description, so their expected behaviour is not completely clear to me.

The easy thing would be to have another option reformat=True, which sets |$ioHook| to nil, and call eval with reformat=False always...

comment:53 Changed 5 years ago by bpage

Well this is a slightly different but related topic: I prefer to treat Sage notebook cells beginning with %fricas like .input files rather than command lines. Because .input language syntax is somewhat more general it allows FriCAS code such as

for i in 1..10 repeat
  if i<5 then
    output i
  else
    output i+1

I notice that you already have code that allows creation and execution of .input files for very long expressions. Making _eval_line always create .input files would not be difficult if you override it in fricas.py.

comment:54 Changed 5 years ago by git

  • Commit changed from 41aefd7bcd18edbbee79a88ba8321e8f689f2fa0 to 63052797a0e42d36ee7c93cdf484506651c789f5

Branch pushed to git repo; I updated commit sha1. New commits:

6305279try to make reviewer happier...

comment:55 Changed 5 years ago by bpage

  • Status changed from needs_work to positive_review

Reviewer is now very happy. :) Maybe we can consider .input format cells in a later ticket.

comment:56 follow-up: Changed 5 years ago by mantepse

There is actually an easy way to achieve what you want: First say:

fricas._eval_using_file_cutoff=30

(I don't know what a good value is. Don't make it 0 or 1.) and then:

%fricas
for i in 100000..100010 repeat
  if i<5 then
    output i
  else
    output partition(i+1)

Note however, that the result will not be displayed one by one, but only after the full thing is finished. I hate that.

comment:57 in reply to: ↑ 56 ; follow-up: Changed 5 years ago by bpage

Replying to mantepse:

There is actually an easy way to achieve what you want: First say:

fricas._eval_using_file_cutoff=30

(I don't know what a good value is. Don't make it 0 or 1.)

Nice! In fact

fricas._eval_using_file_cutoff=1

seems to work for me. Did you have a problem setting it to 1?

I could be happy with just such a flag option, especially if it was a bit more obvious like:

fricas.inputFormat(true)

But why not make it the default? Would you expect a serious performance issue?

Note however, that the result will not be displayed one by one, but only after the full thing is finished. I hate that.

Of course, but this is not a problem in one of the most important uses: defining a long function.

comment:58 in reply to: ↑ 57 Changed 5 years ago by mantepse

Nice! In fact

fricas._eval_using_file_cutoff=1

seems to work for me. Did you have a problem setting it to 1?

Yes, but that might have been noise.

I could be happy with just such a flag option, especially if it was a bit more obvious like:

fricas.inputFormat(true)

Not sure yet, especially about the user interface.

But why not make it the default? Would you expect a serious performance issue?

Yes.

Note however, that the result will not be displayed one by one, but only after the full thing is finished. I hate that.

Of course, but this is not a problem in one of the most important uses: defining a long function.

In case you have time there are two major things that remain to be done (on separate tickets):

  • find out how Waldek's fricas-as-library works.
  • find out how to make fricas._eval_line and fricas.eval print stuff they receive immediately, until a special marker is encountered. (We can control the special marker via |$ioHook|, but for the moment let's pretend it is |startAlgebraOutput|. To be honest, I don't even know whether this is possible.

I don't want to modify this ticket much further, unless the patchbot requires me to do so, to have it merged soon.

comment:59 Changed 5 years ago by git

  • Commit changed from 63052797a0e42d36ee7c93cdf484506651c789f5 to 2231cea79d5c2c11e3520b623c7b5f2a19548b7a
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

1347786Add jump_number() to posets.
3e6ef52<tab> to spaces.
fe815c8Merge branch 'u/jmantysalo/jump_number' of git://trac.sagemath.org/sage into fricas-interface
aa04d96Add algorithm-keyword to canonical labeling of posets.
d923dc1Merge branch 'u/jmantysalo/posets__add_algorithm_keyword_to_canonical_relabel__' of git://trac.sagemath.org/sage into fricas-interface
2231ceabeautify documentation

comment:60 Changed 5 years ago by mantepse

  • Status changed from needs_review to positive_review

I only changed *really* minor documentation stuff.

comment:61 Changed 5 years ago by chapoton

reviewer name must be *full real name*

comment:62 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

comment:63 Changed 5 years ago by mantepse

Is it "needs_work" because of the missing reviewer name?

(the patchbot doesn't like it because it cannot fetch the branch, that's not my fault I believe)

comment:64 Changed 5 years ago by bpage

  • Reviewers changed from bpage to Bill Page
  • Status changed from needs_work to positive_review

I am unsure of the correct protocol when setting/re-setting this "positive review" flag ...

comment:65 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long src/sage/interfaces/fricas.py
**********************************************************************
File "src/sage/interfaces/fricas.py", line 971, in sage.interfaces.fricas.FriCASElement._sage_
Failed example:
    fricas("integrate(sin((x^2+1)/x),x)").sage()
Exception raised:
    Traceback (most recent call last):
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 498, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 861, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.fricas.FriCASElement._sage_[1]>", line 1, in <module>
        fricas("integrate(sin((x^2+1)/x),x)").sage()
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 244, in __call__
        return cls(self, x, name=name)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 1382, in __init__
        raise_(TypeError, x, sys.exc_info()[2])
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 1377, in __init__
        self._name = parent._create(value, name=name)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/interface.py", line 434, in _create
        self.set(name, value)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/fricas.py", line 506, in set
        output = self.eval(cmd, reformat=False)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/fricas.py", line 596, in eval
        **kwds)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 1294, in eval
        for L in code.split('\n') if L != ''])
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 905, in _eval_line
        self._start()
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/fricas.py", line 273, in _start
        Expect._start(self)
      File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/interfaces/expect.py", line 486, in _start
        (self.name(), cmd, e, self._install_hints()))
    TypeError: unable to start fricas because the command 'fricas -nox -noclef' failed: The command was not found or was not executable: fricas.

**********************************************************************
1 item had failures:
   1 of   3 in sage.interfaces.fricas.FriCASElement._sage_
    [3 tests, 1 failure, 0.19 s]

comment:66 Changed 5 years ago by git

  • Commit changed from 2231cea79d5c2c11e3520b623c7b5f2a19548b7a to c651222036f3a60e3d55a52423dc0e325d469e95

Branch pushed to git repo; I updated commit sha1. New commits:

92378caFix for sage shell with TERM=dumb
fc0b081Merge branch 'u/mkoeppe/sage_mode_for_emacs_has_display_problem_in_sage_7_4_beta0' of git://trac.sagemath.org/sage into fricas-interface
c651222fix missing optional in doctests

comment:67 Changed 5 years ago by mantepse

Thanks, fixed. I'm very sorry, but my git-foo is very weak, so there are some commits in the branch which do not belong.

If there is an easy way to get rid of them, please let me know... They are:

​43c1136	Exclude plain text from IPython super call
​b027498	Merge branch 'u/vbraun/call_repr_once' of git://trac.sagemath.org/sage into develop

​1347786	Add jump_number() to posets.
​3e6ef52	<tab> to spaces.
​fe815c8	Merge branch 'u/jmantysalo/jump_number' of git://trac.sagemath.org/sage into fricas-interface
​aa04d96	Add algorithm-keyword to canonical labeling of posets.
​d923dc1	Merge branch 'u/jmantysalo/posets__add_algorithm_keyword_to_canonical_relabel__' of git://trac.sagemath.org/sage into fricas-interface


​92378ca	Fix for sage shell with TERM=dumb
​fc0b081	Merge branch 'u/mkoeppe/sage_mode_for_emacs_has_display_problem_in_sage_7_4_beta0' of git://trac.sagemath.org/sage into fricas-interface

Or should I just create a new branch and copy the good stuff into that new branch?

comment:68 Changed 5 years ago by vbraun

To undo the merge you have to go back to before it:

git reset --hard 2231cea79d5c2c11e3520b623c7b5f2a19548b7a

and then only add the commit you want

git cherry-pick c651222036f3a60e3d55a52423dc0e325d469e95

and then (force) push the new branch

comment:69 Changed 5 years ago by dimpase

https://trac.sagemath.org/ticket/21209#comment:64 shows that you need to implement fricas.quit() here.

comment:70 Changed 5 years ago by mantepse

https://trac.sagemath.org/ticket/21209#comment:66 indicates that it is sufficient to append a \r to the quitstring.

comment:71 Changed 5 years ago by mantepse

  • Branch changed from u/mantepse/21231 to u/mantepse/fricas_interface
  • Commit changed from c651222036f3a60e3d55a52423dc0e325d469e95 to c39474e6b94094d00979ff68aaf7671dc8747d92

comment:72 Changed 5 years ago by git

  • Commit changed from c39474e6b94094d00979ff68aaf7671dc8747d92 to c1baa025052f36a3426ff1e54a3042abb4fbd7ba

Branch pushed to git repo; I updated commit sha1. New commits:

c1baa02remove merge conflict leftover

comment:73 Changed 5 years ago by git

  • Commit changed from c1baa025052f36a3426ff1e54a3042abb4fbd7ba to f6b0b4784999ef24228922dcc823d8d16edd08e9

Branch pushed to git repo; I updated commit sha1. New commits:

f6b0b47indentation fixes

comment:74 Changed 5 years ago by git

  • Commit changed from f6b0b4784999ef24228922dcc823d8d16edd08e9 to 835e519de78fb4650a6fdcdaa21d7a17b2c6ff2f

Branch pushed to git repo; I updated commit sha1. New commits:

835e519try to make the patchbot happy

comment:75 Changed 5 years ago by mantepse

  • Status changed from needs_work to needs_review

comment:76 follow-up: Changed 5 years ago by bpage

There is something strange about the way FriCAS matrices are being typeset.

sage: latex(fricas("matrix([[1,2],[3,4]])"))
 {\left[ 1, \: 2  \right]} \  {\left[ 3, \: 4  \right]} 

but

sage: fricas("matrix([[1,2],[3,4]])::TEX")
["$$", "\left[", "\begin{array}{cc}", "1 & 2 \\ ", "3 & 4 ", "\end{array}",
 "\right]", "$$"]

However in other cases it looks normal. For example:

sage: latex(fricas("integrate(sin(x/(1+sqrt(x))),x)"))
 \int ^{\displaystyle x} {{\sin  \left( {{ \%A \over {{\sqrt { \%A}}+1}}}  \right)} \  {d \%A}}   

sage: fricas("integrate(sin(x/(1+sqrt(x))),x)::TEX")
["$$", "\int \sp{\displaystyle x} {{\sin ", "\left(",
 "{{ \%A \over {{\sqrt { \%A}}+1}}} ", "\right)}", "\  {d \%A}} ", "$$"]

comment:77 in reply to: ↑ 76 Changed 5 years ago by mantepse

Replying to bpage:

There is something strange about the way FriCAS matrices are being typeset.

sage: latex(fricas("matrix([[1,2],[3,4]])"))
 {\left[ 1, \: 2  \right]} \  {\left[ 3, \: 4  \right]} 

but

sage: fricas("matrix([[1,2],[3,4]])::TEX")
["$$", "\left[", "\begin{array}{cc}", "1 & 2 \\ ", "3 & 4 ", "\end{array}",
 "\right]", "$$"]

I am using outputAsTex. Should I use ::TEX instead?

comment:78 Changed 5 years ago by bpage

I am perplexed that outputAsTex does not produce the same thing as ::TEX. I was not aware of that. Perhaps this should be reported as a bug in FriCAS? In any case it seems that the FriCAS command:

)set output tex on

uses the latter method.

(1) -> outputAsTex matrix [[1,2],[3,4]]
$$
{\left[ 1, \: 2 
\right]}
\  {\left[ 3, \: 4 
\right]}
\leqno(1)
$$

                                                                   Type: Void
(2) -> matrix([[1,2],[3,4]])::TEX

   (2)
   ["$$", "\left[", "\begin{array}{cc}", "1 & 2 \\ ", "3 & 4 ", "\end{array}",
    "\right]", "$$"]
                                                              Type: TexFormat
(3) -> )set output tex on
(3) -> matrix([[1,2],[3,4]])     

        +1  2+
   (3)  |    |
        +3  4+
$$
\left[
\begin{array}{cc}
1 & 2 \\ 
3 & 4 
\end{array}
\right]
\leqno(3)
$$

                                                        Type: Matrix(Integer)

I think it is reasonable to expect that typeset mode with the FriCAS interface would produce the same result.

comment:79 Changed 5 years ago by mantepse

ping?

comment:80 Changed 5 years ago by git

  • Commit changed from 835e519de78fb4650a6fdcdaa21d7a17b2c6ff2f to 9a049fc37c5d0bc381ecc58364bc9706485646c5

Branch pushed to git repo; I updated commit sha1. New commits:

102263dMerge branch 'develop' of git://trac.sagemath.org/sage into u/mantepse/fricas_interface
9a049fcimprove latex, various doctest fixes

comment:81 Changed 5 years ago by bpage

Thanks for the update. The output of

sage: latex(fricas("matrix(1,2],[3,4?)")) \left[ \begin{array}{cc} 1 & 2
3 & 4 \end{array} \right]

now looks good but ...

In

sage --notebook

the output from a cell beginning with

%fricas

is no longer being typeset even though the [x]Typeset option is checked.

comment:82 Changed 5 years ago by git

  • Commit changed from 9a049fc37c5d0bc381ecc58364bc9706485646c5 to 601f8fe5eac80f5a4bc4a274e5505091beb7a0c4

Branch pushed to git repo; I updated commit sha1. New commits:

923071bimprove doc
cbcd523Merge branch 'u/mantepse/fricas_interface' of git://trac.sagemath.org/sage into u/mantepse/fricas_interface
601f8fefix bug in latex (and elsewhere), refactor

comment:83 Changed 5 years ago by bpage

  • Status changed from needs_review to positive_review

My comment about how typeset mode works in the a Sage worksheet was incorrect. Everything looks good! Thanks.

comment:84 follow-up: Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:85 in reply to: ↑ 84 Changed 5 years ago by bpage

Replying to vbraun:

Merge conflict

I just did

wspage@strix ~/sage $ git branch
  develop
  master
* t/21231/fricas_interface
wspage@strix ~/sage $ git pull
remote: Counting objects: 1865, done.
remote: Compressing objects: 100% (794/794), done.
remote: Total 1291 (delta 1146), reused 618 (delta 493)
Receiving objects: 100% (1291/1291), 190.57 KiB | 175.00 KiB/s, done.
Resolving deltas: 100% (1146/1146), completed with 398 local objects.
From git://trac.sagemath.org/sage
   0ae5fd8..c5dadf9  develop    -> trac/develop
   62b052e..b57c07f  u/chapoton/21210 -> trac/u/chapoton/21210
   eda9412..c76bdfa  u/chapoton/21523 -> trac/u/chapoton/21523
   41d35fc..9114ec8  u/chapoton/21577 -> trac/u/chapoton/21577
 * [new branch]      u/jdemeyer/gp2c_does_not_pass_self_checks -> trac/u/jdemeyer/gp2c_does_not_pass_self_checks
 + f54ecb5...ccd9c1e u/jdemeyer/pari__use_prot_none_for_unused_virtual_stack_memory -> trac/u/jdemeyer/pari__use_prot_none_for_unused_virtual_stack_memory  (forced update)
 * [new tag]         7.4.beta6  -> 7.4.beta6
Already up-to-date.

and did not see any conflict. How can I see what is wrong?

comment:86 Changed 5 years ago by git

  • Commit changed from 601f8fe5eac80f5a4bc4a274e5505091beb7a0c4 to 08463b9e711e44bf59017a2578e120e37e7eb600

Branch pushed to git repo; I updated commit sha1. New commits:

08463b9Merge branch 'develop' of git://trac.sagemath.org/sage into u/mantepse/fricas_interface

comment:87 Changed 5 years ago by mantepse

  • Status changed from needs_work to positive_review

fixed trivial merge conflict.

comment:88 Changed 5 years ago by git

  • Commit changed from 08463b9e711e44bf59017a2578e120e37e7eb600 to 54cc3ef8e290f7f6af7a24777989f402067ec564
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

54cc3effix a few "::" in documentation.

comment:89 Changed 5 years ago by charpent

  • Reviewers changed from Bill Page to Bill Page Emmanuel Charpentier
  • Status changed from needs_review to positive_review

Passes ptestlong on top of 7.4beta6 (+#21622, mandatory for getting Sage to compile) with one failure :

----------------------------------------------------------------------
sage -t --long --warn-long 113.7 src/sage/homology/simplicial_complex.py  # 1 doctest failed
----------------------------------------------------------------------

which passes standalone :

charpent@asus16-ec:/usr/local/sage-7$ sage -t --long --warn-long 113.7 src/sage/homology/simplicial_complex.py 
Running doctests with ID 2016-10-02-13-43-39-bc494aee.
Git branch: t/21231/fricas_interface
Using --optional=atlas,database_gap,mpir,python2,sage,sage_mode
Doctesting 1 file.
sage -t --long --warn-long 113.7 src/sage/homology/simplicial_complex.py
    [588 tests, 3.20 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 3.3 seconds
    cpu time: 3.0 seconds
    cumulative wall time: 3.2 seconds

This failure is a recurring known problem. ==> positive_review notwithstanding.

comment:90 follow-up: Changed 5 years ago by mantepse

There are a few docbuild errors, I'll correct them tomorrow morning.

comment:91 in reply to: ↑ 90 ; follow-up: Changed 5 years ago by charpent

Replying to mantepse:

There are a few docbuild errors,

Wups. I missed that (how did you find this ?).

I'll correct them tomorrow morning.

Ring me afterwards, I'll re-test it un the evening (European time).

comment:92 in reply to: ↑ 91 Changed 5 years ago by mantepse

Replying to charpent:

Replying to mantepse:

There are a few docbuild errors,

Wups. I missed that (how did you find this ?).

I only saw it on the patchbot report... (there is one one 7.4 beta6 by cristal which seems to report something real)

comment:93 Changed 5 years ago by git

  • Commit changed from 54cc3ef8e290f7f6af7a24777989f402067ec564 to 70c3acede9b908417aea31ab46bd4871ff62a98e
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

70c3acefix docbuild problems and improve doc

comment:94 Changed 5 years ago by mantepse

  • Reviewers changed from Bill Page Emmanuel Charpentier to Bill Page, Emmanuel Charpentier

Everything should be fine now!

comment:95 Changed 5 years ago by charpent

  • Status changed from needs_review to positive_review

Passes ptestlong with the same failure as before, which again passes standalone.

positive_review

comment:96 Changed 5 years ago by git

  • Commit changed from 70c3acede9b908417aea31ab46bd4871ff62a98e to ab4214adf600dbba32abc88c5fcd5d36945a4b14
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

ab4214aMerge branch 'develop' of git://trac.sagemath.org/sage into u/mantepse/fricas_interface

comment:97 Changed 5 years ago by dimpase

the latest push created a mega-patch for no good reason. What was the reason for this?

comment:98 follow-up: Changed 5 years ago by mantepse

I am sorry, I thought I am supposed to merge when a new develop appears. Could you tell me how to revert?

comment:99 in reply to: ↑ 98 ; follow-up: Changed 5 years ago by dimpase

Replying to mantepse:

I am sorry, I thought I am supposed to merge when a new develop appears. Could you tell me how to revert?

only if there are merge conflicts (which you would see by the Branch field in the ticket turning red).

Just revert the last commit (and push again, with -f switch). Not sure how to do this using git trac if you must use it... Let me know if you get stuck on this.

comment:100 Changed 5 years ago by git

  • Commit changed from ab4214adf600dbba32abc88c5fcd5d36945a4b14 to 70c3acede9b908417aea31ab46bd4871ff62a98e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

comment:101 in reply to: ↑ 99 Changed 5 years ago by mantepse

Replying to dimpase:

Thank you!

comment:102 Changed 5 years ago by dimpase

  • Status changed from needs_review to positive_review

OK, good, back to the positively reviewed patch.

comment:103 follow-ups: Changed 5 years ago by jdemeyer

  • Status changed from positive_review to needs_work

You should not use a bare except: (see the end of src/sage/symbolic/integration/external.py). Moreover, I would suggest to replace

     try:
        return result.sage()
     except:
        raise ValueError("Unable to parse: {}".format(result))

by

return result.sage()

comment:104 Changed 5 years ago by git

  • Commit changed from 70c3acede9b908417aea31ab46bd4871ff62a98e to e281742685eb891ff7883bd6e91b16e8c31b92d6

Branch pushed to git repo; I updated commit sha1. New commits:

e281742suggestion by reviewer

comment:105 in reply to: ↑ 103 Changed 5 years ago by mantepse

I agree - I only adapted this part of the code.

comment:106 in reply to: ↑ 103 Changed 5 years ago by mantepse

could you set this back to positive review? I would really like to get this into sage soon, because there will be trivial, but tedious merge conflicts all the time otherwise. I'd be grateful...

comment:107 Changed 5 years ago by jdemeyer

  • Status changed from needs_work to needs_review

Well, if you don't set it to needs_review, I don't know that it's ready for review.

comment:108 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:109 Changed 5 years ago by vbraun

  • Branch changed from u/mantepse/fricas_interface to e281742685eb891ff7883bd6e91b16e8c31b92d6
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.