Opened 11 months ago
Closed 11 months ago
#31085 closed enhancement (fixed)
fix conversion of sign function to fricas
Reported by:  chapoton  Owned by:  

Priority:  major  Milestone:  sage9.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: 
Description
which apparently has no symbolic sign function
Change History (13)
comment:1 Changed 11 months ago by
 Branch set to u/chapoton/31085
 Cc mantepse added
 Commit set to 9cdcb3f9c106e4e93e9a908c22b5676ff560b9be
 Status changed from new to needs_review
comment:2 Changed 11 months ago by
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
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
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
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
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
 Reviewers set to Martin Rubey
comment:8 Changed 11 months ago by
 Keywords FriCAS added
comment:9 Changed 11 months ago by
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
 Commit changed from 9cdcb3f9c106e4e93e9a908c22b5676ff560b9be to 65695c1081d8c8f2f496ac517d61489bd5ad661f
Branch pushed to git repo; I updated commit sha1. New commits:
65695c1  add a doctest for fricas sign

comment:11 Changed 11 months ago by
Voilà !
comment:12 Changed 11 months ago by
 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
 Branch changed from u/chapoton/31085 to 65695c1081d8c8f2f496ac517d61489bd5ad661f
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
fix conversion of sign function to fricas