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

Disable abs_integrate #12731

Closed
roed314 opened this issue Mar 22, 2012 · 50 comments
Closed

Disable abs_integrate #12731

roed314 opened this issue Mar 22, 2012 · 50 comments

Comments

@roed314
Copy link
Contributor

roed314 commented Mar 22, 2012

Maxima's abs_integrate contributed package gives us lots of nice new integrals, but also really breaks a lot of stuff, apparently. Add to this list as you see fit:


Old:

Can someone more knowledgeable about using maxima for integration determine what the right warning to show is and when to show it? See #12691 for what a stopgap is.

CC: @nbruin @orlitzky @kcrisman @pjbruin @rwst @paulmasson @mforets

Component: calculus

Keywords: abs_integrate

Author: Ralf Stephan, Frédéric Chapoton

Branch: b5c9cc5

Reviewer: Dima Pasechnik

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

@roed314 roed314 added this to the sage-5.11 milestone Mar 22, 2012
@kcrisman
Copy link
Member

comment:1

I'm not really sure that this is appropriate for a stopgap. I mean, unless we put in a catch for the specific form like integrate(x * sgn(x^2 - 1/4), x, -1, 0), we would have to put in a stopgap for the entire integration, which is ridiculous.

Unless we checked for the signum function with nonlinear operands in it sometime... but that's more than I have time to parse now.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@kcrisman

This comment has been minimized.

@kcrisman kcrisman changed the title Stopgap for #11590 Disable abs_integrate as default Dec 8, 2014
@kcrisman
Copy link
Member

kcrisman commented Dec 8, 2014

comment:8

Naturally, if it becomes unnecessary to do so, we can just close this as wontfix. This is the best equivalent to a stopgap I can come up with - or possibly we can raise a warning every time abs_integrate is used, though this won't stop the hangs and other weird interactions it causes.

@kcrisman kcrisman changed the title Disable abs_integrate as default Disable or raise warning with abs_integrate as default Dec 8, 2014
@kcrisman

This comment has been minimized.

@kcrisman
Copy link
Member

comment:10

Unfortunately, a lot of the most popular such integrals are not computed with sympy (yet?). Not even abs(x).

In [1]: from sympy import *

In [2]: x=Symbol('x')

In [3]: integrate( abs(x), x)
Out[3]: Integral(Abs(x), x)

In [4]: integrate( abs(x), (x, 0, 1))
Out[4]: Integral(Abs(x), (x, 0, 1))

@kcrisman
Copy link
Member

comment:11

Useful comment from author of abs_integrate:

The assignments extra_definite_integration_methods : [] and extra_integration_methods : [] should render abs_integrate inoperative.
Resorting these lists to their defaults will revive abs_integrate. A kill(all) will remove all traces of abs_integrate--a reload is needed.

So we could conceivably do this in the case that Maxima (or whomever) returns an unevaluated integral, sort of like what we do with solving.

@sagetrac-jakobkroeker
Copy link
Mannequin

sagetrac-jakobkroeker mannequin commented Feb 10, 2015

comment:12

could all developers involved in this ticket eventually meditate on Williams comment:

And why are we using it (abs_integrate) if it is so broken -- it should
require an explicit flag to use at all. Sage is supposed to default
to correct answers unless you otherwise explicitly request otherwise

Does that sound reasonable?

@rwst rwst modified the milestones: sage-6.4, sage-6.6 Mar 7, 2015
@rwst
Copy link

rwst commented Mar 7, 2015

comment:14

Replying to @sagetrac-jakobkroeker:

could all developers involved in this ticket eventually meditate on Williams comment:

And why are we using it (abs_integrate) if it is so broken -- it should
require an explicit flag to use at all. Sage is supposed to default
to correct answers unless you otherwise explicitly request otherwise

Does that sound reasonable?

Yes, and it's the only option because that package causes so much collateral damage, not only in integrate. Turning it off only triggers a few minor doctest failures, most of them (9) in interfaces/maxima_lib.py.

@rwst rwst changed the title Disable or raise warning with abs_integrate as default Disable abs_integrate Mar 7, 2015
@fchapoton
Copy link
Contributor

Changed branch from u/rws/disable_abs_integrate to u/chapoton/12731

@kcrisman
Copy link
Member

comment:44

Thanks - I wish there was a better fix than disabling. Can you briefly comment on

  • The removal of the radexpand example
  • the known bug ones
    I'm sure they are quite logical but a quick glance fails to reveal it, as opposed to where you put the unevaluated integral in to replace the abs_integrate behavior.

Or maybe this was already present behavior and you really did just refresh this, in which case no worries. It seems like comment:33 was the last substantive part of the discussion and memory fails.

@fchapoton
Copy link
Contributor

comment:45

Tagging by known bug allows to remember that one could hope for the answer displayed. I think Jeroen suggested to keep the doctest in this way, rather than remove them.

And the radexpand example no longer works without abs_integrate, just returning unevaluated.

@fchapoton
Copy link
Contributor

comment:46

bot is morally green, please review

@dimpase
Copy link
Member

dimpase commented May 22, 2019

Reviewer: Dima Pasechnik

@dimpase
Copy link
Member

dimpase commented May 22, 2019

comment:47

looks good to me. Please add yourself as authors/reviewers...

@fchapoton
Copy link
Contributor

Changed author from Ralf Stephan to Ralf Stephan, Frédéric Chapoton

@vbraun
Copy link
Member

vbraun commented May 24, 2019

Changed branch from u/chapoton/12731 to b5c9cc5

@kcrisman
Copy link
Member

Changed commit from b5c9cc5 to none

@kcrisman
Copy link
Member

comment:50

At least according to github, the following stuff in sage/maxima_lib.py was not removed (probably because it still passes doctests as Maxima proper improved in the meantime):

        Make sure the abs_integrate package is being used,
        :trac:`11483`. The following are examples from the Maxima
        abs_integrate documentation::
            sage: integrate(abs(x), x)
            1/2*x*abs(x)

@embray
Copy link
Contributor

embray commented Jul 3, 2019

comment:51

Not in Sage 8.8. Let's please to try keep tickets' milestones related to the release in which we actually intend to include them, and in particular the release in which they were actually included, especially when closing tickets.

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