Opened 6 years ago

Closed 6 years ago

#16732 closed defect (fixed)

Fix _maxima_init_evaled_ use, as well as translations of psi etc. to/from maxima.

Reported by: nbruin Owned by:
Priority: major Milestone: sage-6.3
Component: interfaces Keywords:
Cc: rws, kcrisman, burcin Merged in:
Authors: Nils Bruin Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: c28d9bd (Commits) Commit: c28d9bdcb7ed0023f5eccb31d021207668259ee2
Dependencies: Stopgaps:

Description (last modified by nbruin)

Follow-up to #9217. We have some imperfect translations to/from maxima of psi(x,y). The template is:

sage: x,y=var("x y")
sage: maxima_calculus(psi(x,y))

The conversion doesn't seem to recurse on either argument, though:

sage: maxima_calculus(psi(psi(x,y),y))
sage: maxima_calculus(psi(x,psi(x,y)))

(note that the inner psi does not have the square brackets in either case.) Perhaps even worse:

sage: maxima_calculus(psi(x,polylog(3,x)))
sage: maxima_calculus(polylog(3,x))

As you see, the polylog doesn't get translated properly! This indicates misimplemented _maxima_init_evaled_, and probably those occur in several spots. Perhaps the way _maxima_init_evaled_ gets called by the translator needs amendment? (the arguments need processing). The problem seems to occur in sage/symbolic/expression_conversion line 550, where the arguments are fed raw into _maxima_init_evaled_ (compare it to the cases below where the operands do get passed through the interface) and where _maxima_init_evaled_ is usually implemented as some basic string manipulation.

Round-tripping only works for completely numeric first arguments (as per how the responsible regexp is designed):

sage: maxima_calculus(psi(x,y))._sage_()
TypeError: unable to make sense of Maxima expression 'psi[x](y)' in Sage

These conversions all go fine with the tree-walking translation of maxima_lib (which gets used for some, but not all calculus operations):

sage: from sage.interfaces.maxima_lib import sr_to_max,max_to_sr
sage: T=maxima_calculus(sr_to_max(psi(psi(x,y),y))); T
sage: max_to_sr(T.ecl())
psi(psi(x, y), y)

Note that the regex-based "._sage_()" conversion will fundamentally have trouble with nested square brackets:

sage: T._sage_()
TypeError: unable to make sense of Maxima expression 'psi[psi[x](y)](y)' in Sage

As a first step we might want to change the regex to accept "non-]" inside the square brackets to allow translation with non-numeric parameters. I don't think anybody has seen an expression in the wild yet that has nested square brackets (indeed, mostly the parameter is explicit numeric).

There are some other maxima functions that use square brackets too. These might benefit from a similar treatment.

Change History (9)

comment:1 Changed 6 years ago by nbruin

  • Description modified (diff)

comment:2 Changed 6 years ago by nbruin

  • Cc kcrisman burcin added
  • Priority changed from minor to major
  • Summary changed from Fix translation of psi(x,y) to/from maxima to Fix _maxima_init_evaled_ use, as well as translations of psi etc. to/from maxima.

turns out the issue here lies a little deeper and can really affect conceivably useful expressions, so I'm upgrading to usual "major" priority.

comment:3 Changed 6 years ago by nbruin

OK, little search:

sage: search_src("init_evaled")
functions/    def _maxima_init_evaled_(self, *args):
functions/            sage: f._maxima_init_evaled_(1/2, 1/2)
functions/        return parent(maxima("%s, numer"%self._maxima_init_evaled_(*args)))
functions/            s = maxima(self._maxima_init_evaled_(*args))

the above are fine. They process the arguments.

functions/    def _maxima_init_evaled_(self, *args):
functions/    def _maxima_init_evaled_(self, n, z):
functions/    def _maxima_init_evaled_(self, n, x):

The above are bad. They do not process their arguments.

functions/    def _maxima_init_evaled_(self, *args):
functions/            sage: P._maxima_init_evaled_(2, 5) is None
functions/    def _maxima_init_evaled_(self, n, x):
functions/            sage: chebyshev_T._maxima_init_evaled_(1,x)
functions/    def _maxima_init_evaled_(self, n, x):

These are fine.

So the only problems are in polylog, lambert_w, psi. They should just call _maxima_init_() on all their arguments before stuffing them in the string template.

comment:4 Changed 6 years ago by nbruin

For the bracket problem: since we are *removing* the square brackets we CAN actually do the transformations with regexes, if we apply the substitutions repeatedly:

sage: s="psi[psi[a](b)](c)"
sage: s=maxima_polygamma.sub('psi(\g<1>,',s);s
sage: s=maxima_polygamma.sub('psi(\g<1>,',s);s

so we could have a little square-bracket replacement loop:

sold = None
while s != sold:
    sold = s
    s=maxima_<other square bracket pattern>.sub(...,s)
    s=maxima_<yet other one>.sub(...,s)

we should converge to a fixed point that is square-bracketless if our patterns cover all legitimate uses of square brackets: If there is a square-bracket use, there is an "innermost" one (that doesn't contain any square bracket uses itself) and its pattern will trigger and remove it (modifying s).

I'm not claiming this is an *efficient* way of doing it, but it can be done with relatively mild changes to the code.

comment:5 Changed 6 years ago by nbruin

  • Branch set to u/nbruin/ticket/16732

comment:6 Changed 6 years ago by nbruin

  • Commit set to c28d9bdcb7ed0023f5eccb31d021207668259ee2
  • Status changed from new to needs_review

Something along these lines should do the job.

New commits:

c28d9bdtrac #16732: make _maxima_init_evaled_ implementations and some bracket translations more robust

comment:7 Changed 6 years ago by nbruin

  • Description modified (diff)

comment:8 Changed 6 years ago by rws

  • Authors set to Nils Bruin
  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

Looks straightforward and patchbot is happy.

comment:9 Changed 6 years ago by vbraun

  • Branch changed from u/nbruin/ticket/16732 to c28d9bdcb7ed0023f5eccb31d021207668259ee2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.