#31085 closed enhancement (fixed)

fix conversion of sign function to fricas

Reported by: chapoton Owned by:
Priority: major Milestone: sage-9.3
Component: interfaces: optional Keywords: FriCAS
Cc: mantepse Merged in:
Authors: Frédéric Chapoton Reviewers: Martin Rubey
Report Upstream: N/A Work issues:
Branch: 65695c1 (Commits, GitHub, GitLab) Commit: 65695c1081d8c8f2f496ac517d61489bd5ad661f
Dependencies: Stopgaps:

Status badges

Description

which apparently has no symbolic sign function

Change History (13)

comment:1 Changed 11 months ago by chapoton

  • Branch set to u/chapoton/31085
  • Cc mantepse added
  • Commit set to 9cdcb3f9c106e4e93e9a908c22b5676ff560b9be
  • Status changed from new to needs_review

New commits:

9cdcb3ffix conversion of sign function to fricas

comment:2 Changed 11 months ago by mantepse

I don't quite understand the patch. If you want a sign function for symbolic expressions in fricas, you'll have to use x/abs(x) , I guess. But I don't think that fricas would be able to do anything with it. There is a sign function for integers and integer fractions, but I don't see the point of providing a translation, because it should never show up in a result that fricas provides.

comment:3 Changed 11 months ago by chapoton

I was trying the integral in #11590 with fricas, and it did not work. The existing conversion of the symbolic sign function is "sign", which cannot handle symbolic arguments.

But then apparently, Fricas cannot integrate functions containing abs.

comment:4 Changed 11 months ago by mantepse

Yes, that's correct (as far as I know).

So, do you think it makes sense to convert sign to x/abs(x)? (I don't)

comment:5 Changed 11 months ago by chapoton

Well, at least by converting to abs(x)/x (I prefer this way, and did that), one can go to fricas and back to sage without error. Why do you think that it's not a good idea ?

comment:6 Changed 11 months ago by mantepse

Sorry, I missed that! Yes, I agree with that! A doctest might be good, I'll try to think of one.

comment:7 Changed 11 months ago by mantepse

  • Reviewers set to Martin Rubey

comment:8 Changed 11 months ago by mantepse

  • Keywords FriCAS added

comment:9 Changed 11 months ago by mantepse

Here is a trival test, do you want to add it?

sage: fricas(sign(x)).eval(x=-3)
- 1

Other than that, it looks good. There is currently a giac failure, which I cannot say anything about.

comment:10 Changed 11 months ago by git

  • Commit changed from 9cdcb3f9c106e4e93e9a908c22b5676ff560b9be to 65695c1081d8c8f2f496ac517d61489bd5ad661f

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

65695c1add a doctest for fricas sign

comment:11 Changed 11 months ago by chapoton

Voilà !

comment:12 Changed 11 months ago by mantepse

  • Status changed from needs_review to positive_review

Great, looks good. I cannot reproduce the giac failure, I guess it's unrelated.

comment:13 Changed 11 months ago by vbraun

  • Branch changed from u/chapoton/31085 to 65695c1081d8c8f2f496ac517d61489bd5ad661f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.