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

improve FriCAS interface #21231

Closed
mantepse opened this issue Aug 12, 2016 · 139 comments
Closed

improve FriCAS interface #21231

mantepse opened this issue Aug 12, 2016 · 139 comments

Comments

@mantepse
Copy link
Contributor

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.

Depends on #21209

CC: @sagetrac-bpage @hemmecke @dkrenn @dimpase

Component: interfaces

Keywords: FriCAS

Author: Martin Rubey

Branch/Commit: e281742

Reviewer: Bill Page, Emmanuel Charpentier

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

@mantepse mantepse added this to the sage-7.4 milestone Aug 12, 2016
@mantepse
Copy link
Contributor Author

comment:1
  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.

  1. 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<sup>n/y</sup>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)]

@mantepse
Copy link
Contributor Author

Commit: 5b7e6ab

@mantepse
Copy link
Contributor Author

Branch: /u/mantepse/21231

@mantepse
Copy link
Contributor Author

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

@mantepse
Copy link
Contributor Author

comment:4

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

@mantepse
Copy link
Contributor Author

Author: Martin Rubey

@mantepse
Copy link
Contributor Author

Changed commit from 5b7e6ab to d8a26e1

@mantepse
Copy link
Contributor Author

New commits:

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

@vbraun
Copy link
Member

vbraun commented Aug 13, 2016

Changed branch from /u/mantepse/21231 to u/mantepse/21231

@vbraun
Copy link
Member

vbraun commented Aug 13, 2016

comment:6

Branch name doesn't have leading slash


New commits:

b2ee8ccmake it build

@vbraun
Copy link
Member

vbraun commented Aug 13, 2016

Changed commit from d8a26e1 to b2ee8cc

@mantepse
Copy link
Contributor Author

comment:7

Replying to @vbraun:

Branch name doesn't have leading slash

Howto? I used

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

@dimpase
Copy link
Member

dimpase commented Aug 14, 2016

comment:8

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.

@dimpase
Copy link
Member

dimpase commented Aug 14, 2016

Dependencies: #21209

@mantepse
Copy link
Contributor Author

comment:10

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2016

Changed commit from b2ee8cc to cbcb94f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 14, 2016

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

@sagetrac-bpage
Copy link
Mannequin

sagetrac-bpage mannequin commented Aug 15, 2016

Sage worksheet under 7.2

@sagetrac-bpage
Copy link
Mannequin

sagetrac-bpage mannequin commented Aug 15, 2016

Attachment: sage-7.2-fricas.pdf.gz

Attachment: sage-7.4.beta0-fricas.pdf.gz

Sage worksheet under 7.4.beta0 (typeset output fails)

@sagetrac-bpage
Copy link
Mannequin

sagetrac-bpage mannequin commented Aug 15, 2016

comment:12

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

https://github.com/sagemath/sage-prod/files/10658907/sage-7.2-fricas.pdf.gz (typeset output OK)

https://github.com/sagemath/sage-prod/files/10658908/sage-7.4.beta0-fricas.pdf.gz (typeset output fails)

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 15, 2016

Changed commit from cbcb94f to 5b4a95d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 15, 2016

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

5b4a95dswitch to better type treatment

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2016

Changed commit from 5b4a95d to 0ab0f92

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2016

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

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

@mantepse
Copy link
Contributor Author

comment:15

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_

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2016

Changed commit from 0ab0f92 to d2d0fb8

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 10, 2016

Changed commit from 70c3ace to ab4214a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 10, 2016

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

@dimpase
Copy link
Member

dimpase commented Oct 10, 2016

comment:97

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

@mantepse
Copy link
Contributor Author

comment:98

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

@dimpase
Copy link
Member

dimpase commented Oct 10, 2016

comment:99

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 10, 2016

Changed commit from ab4214a to 70c3ace

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 10, 2016

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

@mantepse
Copy link
Contributor Author

comment:101

Replying to @dimpase:

Thank you!

@dimpase
Copy link
Member

dimpase commented Oct 10, 2016

comment:102

OK, good, back to the positively reviewed patch.

@jdemeyer
Copy link

comment:103

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()

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2016

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

e281742suggestion by reviewer

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 12, 2016

Changed commit from 70c3ace to e281742

@mantepse
Copy link
Contributor Author

comment:105

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

@mantepse
Copy link
Contributor Author

comment:106

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

@jdemeyer
Copy link

comment:107

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

@vbraun
Copy link
Member

vbraun commented Oct 21, 2016

Changed branch from u/mantepse/fricas_interface to e281742

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

6 participants