On Tue, May 21, 2013 at 4:41 AM, Chris Smith <smi...@gmail.com> wrote: > >> Is there a good reason for this, though? If we changed the solve >> subalgorithms to catch NotImplementedError and make them just give [] >> in that case, wouldn't solve(cos(x)*x - x**2, x) give [0], more or >> less automatically? Doesn't the algorithm look something like >> >> solutions = [] >> for factor in split_apart_multiplicatively(expr): >> solutions.extend(relevant_solving_algorithm(factor)) >> return solutions >> > > If it's a Mul, yes. But if it hits a NotImplemented it chokes. So in this > sense when you are getting a solution from solve you are getting everything > (we presently know how to solve...almost: I know that 2 branches from > lambert solutions are not being returned and as I commented on the issue > about solve not returning all solutions, I'm not sure how one will *know* > this anyway so I don't know how to assure anyone that solve has found all > the solutions).
I think these are unrelated things. I am suggesting to catch the NotImplementedError, and just give the solutions it did find. Regarding finding all the solutions, we need to come up with a good way to pass this information through. When we do things like catch NotImplementedError or go through solvers that are just heuristics, we will set the flag to False. But not having everything is no reason to return nothing. > >> (plus all the other stuff, like getting the numerator and checking and so >> on)? >> >> By the way, are you willing to work on this? You know more about the >> architecture of solve() than anyone else. > > > I can't get into this now. I just spent a weekend+ fixing two "simple" > things and this is going to be more involved than that. We'll see if I have time. I may not, as I am starting my internship soon. > >> >> In particular, for the >> NotImplementedError, I am not sure just how much of the code needs to >> be wrapped in try, except. My solution would just be to refactor >> solve into _solve, > > > (or equivalent since that's already been used) Oh, I didn't notice this. solve() is just preprocessing, but there is so much of it that I assumed it was everything. > >> >> and wrap that in try, except, but that's just the >> lazy way. We could also try to change all upstream algorithms to not >> raise NotImplementedError, but that would be a lot harder, and we'd be >> liable to miss some stuff. >> >> For dict=True, I could probably work it out, since it's mainly just a >> matter of changing library code that calls solve(). Do you want to >> work on it, or should I? Which direction do you think we should take? >> Should we make dict=True the default for everything, or only for >> systems? dict=True is already the format used for systems if no >> variables are given, so it's less of a compatibility break. For single >> equations, it would be a bigger break, but if you think it would be >> best to use it there too, I am in support of it. >> > > This is where you better get some consensus before starting. This has been > the reason that I didn't do more than add the `set` and `dict` flags to > solver. It's just not worth the investment unless there is a community > commitment to review and commit the changes before they become stale and in > need of rebasing. I agree. One issue is that this would just be a temporary fix. The real fix would be Solution. But if we can agree that Solution and SolutionSet should keep the same interface as a list of dicts, then it won't be a big deal. We could also just implemented Solution already. I think the biggest thing stopping that is we don't yet know what it should look like. > > Note, too, that linear cases come out as a simple dict rather than a dict > wrapped in a list. If you just change the False in `flags.get('dict') = > False` to True and run the doctests on solvers.py you will see the issues > that need to be decided. I did that for only systems case (only if f is iterable), and ran the regular tests, and there were quite a few failures. It seems that more library code solves systems than I thought. I didn't check further, but I suspect that at least some of it is wrong, in the sense that it doesn't check for both possibilities. > >> For the corner cases, like some algorithm returning a generator, or >> raising an exception other than NotImplementedError (issue 3686), >> these are bugs, which are easy enough to fix. >> > > That's fixed in a PR needing review. Thanks. Aaron Meurer > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to sympy+unsubscr...@googlegroups.com. > To post to this group, send email to sympy@googlegroups.com. > Visit this group at http://groups.google.com/group/sympy?hl=en-US. > For more options, visit https://groups.google.com/groups/opt_out. > > -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to sympy+unsubscr...@googlegroups.com. To post to this group, send email to sympy@googlegroups.com. Visit this group at http://groups.google.com/group/sympy?hl=en-US. For more options, visit https://groups.google.com/groups/opt_out.