Modify subs so that it can accept multiple equations just like subs_expr #12834

sagetrac-JoalHeagney · 2012-04-12T13:03:22Z

In this ticket:

we make expr.substitute(in_dict, **kwds) as expr.substitute(*args, **kwds) and make it accepts any kind of arguments (symbolic equalities, dictionnaries or lists of them)
we deprecate subs_expr as a duplicate of subs

This came up as a method of passing solutions from solve back into symbolic equations:

    f(x,y) = (1-x)^2 + 100*(y - x^2)^2
    solution = solve([f.diff(ar) for ar in f.args()],f.args())[0]

Gives me a list of solutions as:

    [x=1;y=1]

Is there any way to programatically substitute this list of equalities back into f?

The solution provided by Michael Orlitzky was

f(x,y).subs_expr(*solution)

Anycase, as noted by Michael, it would be nice if subs had the same behaviour:

Passing it one equation does work,

  sage: f.subs(x == 1)
  y + z + 1

But more than one doesn't,

  sage: f.subs(x == 1, y == 2)
  ...
  TypeError: substitute() takes at most 1 positional argument (2 given)

I guess all that's missing is the ability to pass it multiple equations,
like `subs_expr`. It would probably be easy to add that ability to
`subs` if you want to create a ticket for something.

With the branch applied we have:

sage: f(x,y) = (1-x)^2 + 100*(y - x^2)^2
sage: solution = solve([f.diff(ar) for ar in f.args()],f.args())
sage: solution
[[x == 1, y == 1]]
sage: f.subs(solution[0])
(x, y) |--> 0

CC: @kcrisman

Component: symbolics

Keywords: subs algebra solving

Author: Michael Orlitzky, Vincent Delecroix

Branch/Commit: cb5347e

Reviewer: Vincent Delecroix, Michael Orlitzky

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

The text was updated successfully, but these errors were encountered:

nbruin · 2012-04-12T19:37:30Z

comment:1

subs_expr is already just doing a bit of preprocessing and then calling subs, so if the interface of subs is changed to accept a wider variety of input then subs_expr can simply be done away with. I'd say

subs(self,*args,**kwargs):
   for k in args:
       if is_symbolic_equation(k):
            kwargs[k.lhs()]=k.rhs() # but signal an error if key k is already present
       elif is_dict(k):
            add entries of k to kwargs # but signal error any key in k is already in kwargs.
       elif is_iterable(k):
            for e in k:
                if is_symbolic_equation(e):
                    same as before
                else:
                    error
       else:
            error
   ...

Note that behaviour at the moment is a bit random:

sage: x.subs({x:1})
1
sage: x.subs({x:1},x=2)
2
sage: x.subs(x==1,x=2)
1
sage: x.subs(x=1,x=2)
SyntaxError: keyword argument repeated

sagetrac-JoalHeagney · 2012-04-13T01:42:19Z

comment:2

I would argue that we accept the weird behaviour in the cases above, but document it.

Most users are (hopefully) unlikely to use such mixed arguments, and trying to predict the desired behaviour of those that do will probably result in the wrong answer.

orlitzky · 2012-04-13T06:37:41Z

Attachment: sage-trac_12834.patch.gz

Make Expression.subs() accept lists, throw errors on subs conflicts, and doc cleanup.

orlitzky · 2012-04-13T06:43:37Z

comment:3

Some Cython limitations made this more of an event than it had to be, but the patch implements the requested feature and Nils' suggestions from comment 1.

When you pass subs() a list, it's processed recursively, which has the nice side effect of allowing you to call solve() and pass the result to subs() without caring about the form of the return value.

The new subs() throws an error on conflicting substitutions... nobody reads the documentation unless they have to so I think it's better to alert the user that he's likely making a mistake.

sagetrac-JoalHeagney · 2012-06-09T04:45:31Z

comment:5

Well I tried the patch out on my example and it seems to work well.

vbraun · 2013-02-24T03:52:28Z

comment:6

Patch needs to be rediffed for current Sage...

orlitzky · 2013-02-25T05:28:43Z

comment:7

Attachment: sage-trac_12834.2.patch.gz

Updated!

videlec · 2015-04-26T17:52:45Z

Commit: 045eebf

videlec · 2015-04-26T17:52:45Z

comment:12

Hello,

I updated the patch as a git branch (trivial conflict) and I propose some cleanup in a second commit. Tell me what you think.

Vincent

New commits:

`447b35d`	`Trac #12834: Allow Expression.subs() to take a list similarly to Expression.subs_expr().`
`045eebf`	`Trac #12384: simplification and cleanup`

videlec · 2015-04-26T17:52:45Z

Branch: public/12384

mezzarobba · 2015-04-27T08:37:16Z

comment:14

As it happens, I had started doing my own version of something similar a while ago at a time where I had no internet access and hence without seeing this ticket. The implementation here is probably better, but I pushed mine to u/mmezzarobba/subs in case somebody wants to see if there is anything to be salvaged. (I may doit myself in a few days/weeks, but I have no time right now.)

videlec · 2015-05-08T16:52:48Z

comment:15

Hi Marc,

There is some difference of designs.

in your version, the rightmost occurrence of a substitution takes precedence, while in Michael's one an error is raised
you deprecate subs_expr and substitute_expression (I think this is good)
is this a bug or a feature?

sage: var('x,y,t')
sage: A = cos(x) + sin(y) + x^2 + y^2 + t
sage: A.subs({x^2 + y^2: t})
x^2 + y^2 + t + cos(x) + sin(y)
sage: A.subs({cos(x) + x^2 + sin(y) + t + + y^2: 1})
1

I like very much the presence of examples involving maple and mathematica

Vincent

sagetrac-git · 2015-05-11T19:13:38Z

Changed commit from e195b36 to 33fd449

sagetrac-git · 2015-05-11T19:13:38Z

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

`33fd449`	`Trac #12834: Fix hash lookup in _dict_update_check_duplicate.`

videlec · 2015-05-11T19:21:56Z

comment:35

Replying to @orlitzky:

Replying to @videlec:

The first any(k in d1 for k in d2) is probably O(m*n), since it (potentially) has to look through all of both dictionaries to see if there are any duplicates. Then,

You are wrong. A dictionary is a hash table not a list. Assuming that there is no collision this is a O(m) where m=size(d2).

Right, but doesn't this
k in d1 for k in d2
do the d1[k] lookup (which is O(n)) m times, once for each key in d2?

The lookup is O(1). And once for each key in d2 gives O(m). There is no for loop on d1, isn't it?

videlec · 2015-05-11T19:22:54Z

comment:36

Looks good to me. Can I add a commit to show a more relevant example about comparisons of maple/mathematica with respect to substitution?

orlitzky · 2015-05-11T19:44:20Z

comment:37

Replying to @videlec:

The lookup is O(1). And once for each key in d2 gives O(m). There is no for loop on d1, isn't it?

Ah, I was using the worst-case O(n) instead of the average O(1).

Looks good to me. Can I add a commit to show a more relevant example about comparisons of maple/mathematica with respect to substitution?

Sure.

videlec · 2015-05-17T21:38:18Z

comment:38

Sorry for the delay!

I added a more complete example comparing maxima/maple/mathematica. Tested with the last version of maple but I do not have access to Mathematica. So it lacks some test.

I think I am done. So I let you review my last commit.

note: it is based on 6.7.rc0

Vincent

Last 10 new commits:

`e8b9c05`	`Trac 12834: review`
`81ebcdc`	`Trac 12834: fix the french book that is using subs_expr`
`f7a233b`	`Trac #12834: Avoid iterating over the substitution dict twice.`
`322981e`	`Trac #12834: Ensure consistent failures by sorting duplicate substitutions.`
`003fa00`	`Trac #12834: Test equality of dictionaries to avoid sorting bugs.`
`5460cfc`	`Trac #12834: Mention the tuple representation in _subs_make_dict's docstring.`
`e617094`	`Trac #12834: Manually word-wrap some lines.`
`37aa1f8`	`Trac #12834: Simplify duplicate detection in _dict_update_check_duplicate.`
`d2d58a3`	`Trac #12834: Fix hash lookup in _dict_update_check_duplicate.`
`a56c969`	`Trac 12834: more details substitution examples`

videlec · 2015-05-17T21:38:18Z

Changed commit from 33fd449 to a56c969

videlec · 2015-05-17T21:38:18Z

Changed branch from u/mjo/ticket/12834 to u/vdelecroix/12834

orlitzky · 2015-05-19T00:46:23Z

Changed branch from u/vdelecroix/12834 to u/mjo/ticket/12834

orlitzky · 2015-05-19T00:46:23Z

Changed commit from a56c969 to 66b961c

orlitzky · 2015-05-19T00:46:23Z

comment:39

I made some minor cosmetic fixes (typo fix and word-wrap). Other than that it looks good to me. As long as those optional tests work (I don't have maple/mathematica), you can mark it positive review.

Last 10 new commits:

`81ebcdc`	`Trac 12834: fix the french book that is using subs_expr`
`f7a233b`	`Trac #12834: Avoid iterating over the substitution dict twice.`
`322981e`	`Trac #12834: Ensure consistent failures by sorting duplicate substitutions.`
`003fa00`	`Trac #12834: Test equality of dictionaries to avoid sorting bugs.`
`5460cfc`	`Trac #12834: Mention the tuple representation in _subs_make_dict's docstring.`
`e617094`	`Trac #12834: Manually word-wrap some lines.`
`37aa1f8`	`Trac #12834: Simplify duplicate detection in _dict_update_check_duplicate.`
`d2d58a3`	`Trac #12834: Fix hash lookup in _dict_update_check_duplicate.`
`a56c969`	`Trac 12834: more details substitution examples`
`66b961c`	`Trac #12834: Typo fix and manually word-wrap a few doctests.`

videlec · 2015-05-19T07:10:04Z

comment:40

Thanks for the last fixes!

vbraun · 2015-05-19T20:06:58Z

comment:41

sage -t --long --warn-long 26.6 src/sage/doctest/sources.py
**********************************************************************
File "src/sage/doctest/sources.py", line 694, in sage.doctest.sources.FileDocTestSource._test_enough_doctests
Failed example:
    for path, dirs, files in itertools.chain(os.walk('sage'), os.walk('doc')): # long time
        path = os.path.relpath(path)
        dirs.sort(); files.sort()
        for F in files:
            _, ext = os.path.splitext(F)
            if ext in ('.py', '.pyx', '.pxd', '.pxi', '.sage', '.spyx', '.rst'):
                filename = os.path.join(path, F)
                FDS = FileDocTestSource(filename, DocTestDefaults(long=True,optional=True))
                FDS._test_enough_doctests(verbose=False)
Expected:
    There are 7 tests in sage/combinat/dyck_word.py that are not being run
    There are 4 tests in sage/combinat/finite_state_machine.py that are not being run
    There are 6 tests in sage/combinat/interval_posets.py that are not being run
    There are 18 tests in sage/combinat/partition.py that are not being run
    There are 15 tests in sage/combinat/permutation.py that are not being run
    There are 14 tests in sage/combinat/skew_partition.py that are not being run
    There are 18 tests in sage/combinat/tableau.py that are not being run
    There are 8 tests in sage/combinat/crystals/tensor_product.py that are not being run
    There are 11 tests in sage/combinat/rigged_configurations/rigged_configurations.py that are not being run
    There are 15 tests in sage/combinat/root_system/cartan_type.py that are not being run
    There are 8 tests in sage/combinat/root_system/type_A.py that are not being run
    There are 8 tests in sage/combinat/root_system/type_G.py that are not being run
    There are 3 unexpected tests being run in sage/doctest/parsing.py
    There are 1 unexpected tests being run in sage/doctest/reporting.py
    There are 9 tests in sage/graphs/graph_plot.py that are not being run
    There are 3 tests in sage/rings/invariant_theory.py that are not being run
Got:
    There are 7 tests in sage/combinat/dyck_word.py that are not being run
    There are 4 tests in sage/combinat/finite_state_machine.py that are not being run
    There are 6 tests in sage/combinat/interval_posets.py that are not being run
    There are 18 tests in sage/combinat/partition.py that are not being run
    There are 15 tests in sage/combinat/permutation.py that are not being run
    There are 14 tests in sage/combinat/skew_partition.py that are not being run
    There are 18 tests in sage/combinat/tableau.py that are not being run
    There are 8 tests in sage/combinat/crystals/tensor_product.py that are not being run
    There are 11 tests in sage/combinat/rigged_configurations/rigged_configurations.py that are not being run
    There are 15 tests in sage/combinat/root_system/cartan_type.py that are not being run
    There are 8 tests in sage/combinat/root_system/type_A.py that are not being run
    There are 8 tests in sage/combinat/root_system/type_G.py that are not being run
    There are 3 unexpected tests being run in sage/doctest/parsing.py
    There are 1 unexpected tests being run in sage/doctest/reporting.py
    There are 9 tests in sage/graphs/graph_plot.py that are not being run
    There are 3 tests in sage/rings/invariant_theory.py that are not being run
    There are 10 tests in sage/symbolic/expression.pyx that are not being run

videlec · 2015-05-20T10:52:16Z

comment:42

My mistake. Sorry.

New commits:

`cb5347e`	`Trac 12834: move doctests related to the deprecated subs_expr`

videlec · 2015-05-20T10:52:16Z

Changed commit from 66b961c to cb5347e

videlec · 2015-05-20T10:52:16Z

Changed branch from u/mjo/ticket/12834 to u/vdelecroix/12834

videlec · 2015-05-20T19:05:03Z

comment:44

This time all tests pass.

vbraun · 2015-05-20T22:28:21Z

Changed branch from u/vdelecroix/12834 to cb5347e

sagetrac-JoalHeagney mannequin added this to the sage-5.11 milestone Apr 12, 2012

sagetrac-JoalHeagney mannequin added c: algebra labels Apr 12, 2012

sagetrac-JoalHeagney mannequin assigned aghitza Apr 12, 2012

orlitzky added the s: needs review label Apr 13, 2012

This comment has been minimized.

Sign in to view

jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013

sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014

sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014

sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014

This comment has been minimized.

Sign in to view

videlec added s: needs info and removed s: needs review labels May 8, 2015

videlec added s: needs review and removed s: needs work labels May 17, 2015

videlec added s: positive review and removed s: needs review labels May 19, 2015

vbraun added s: needs work and removed s: positive review labels May 19, 2015

videlec added s: positive review and removed s: needs work labels May 20, 2015

videlec modified the milestones: sage-6.4, sage-6.8 May 20, 2015

vbraun removed the s: positive review label May 20, 2015

vbraun closed this as completed in ada387a May 20, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify subs so that it can accept multiple equations just like subs_expr #12834

Modify subs so that it can accept multiple equations just like subs_expr #12834

sagetrac-JoalHeagney mannequin commented Apr 12, 2012

nbruin commented Apr 12, 2012

sagetrac-JoalHeagney mannequin commented Apr 13, 2012

orlitzky commented Apr 13, 2012

orlitzky commented Apr 13, 2012

sagetrac-JoalHeagney mannequin commented Jun 9, 2012

vbraun commented Feb 24, 2013

orlitzky commented Feb 25, 2013

This comment has been minimized.

videlec commented Apr 26, 2015

videlec commented Apr 26, 2015

videlec commented Apr 26, 2015

This comment has been minimized.

mezzarobba commented Apr 27, 2015

videlec commented May 8, 2015

sagetrac-git mannequin commented May 11, 2015

sagetrac-git mannequin commented May 11, 2015

videlec commented May 11, 2015

videlec commented May 11, 2015

orlitzky commented May 11, 2015

videlec commented May 17, 2015

videlec commented May 17, 2015

videlec commented May 17, 2015

orlitzky commented May 19, 2015

orlitzky commented May 19, 2015

orlitzky commented May 19, 2015

videlec commented May 19, 2015

vbraun commented May 19, 2015

videlec commented May 20, 2015

videlec commented May 20, 2015

videlec commented May 20, 2015

videlec commented May 20, 2015

vbraun commented May 20, 2015

Modify subs so that it can accept multiple equations just like subs_expr #12834

Modify subs so that it can accept multiple equations just like subs_expr #12834

Comments

sagetrac-JoalHeagney mannequin commented Apr 12, 2012

nbruin commented Apr 12, 2012

sagetrac-JoalHeagney mannequin commented Apr 13, 2012

orlitzky commented Apr 13, 2012

orlitzky commented Apr 13, 2012

sagetrac-JoalHeagney mannequin commented Jun 9, 2012

vbraun commented Feb 24, 2013

orlitzky commented Feb 25, 2013

This comment has been minimized.

videlec commented Apr 26, 2015

videlec commented Apr 26, 2015

videlec commented Apr 26, 2015

This comment has been minimized.

mezzarobba commented Apr 27, 2015

videlec commented May 8, 2015

sagetrac-git mannequin commented May 11, 2015

sagetrac-git mannequin commented May 11, 2015

videlec commented May 11, 2015

videlec commented May 11, 2015

orlitzky commented May 11, 2015

videlec commented May 17, 2015

videlec commented May 17, 2015

videlec commented May 17, 2015

orlitzky commented May 19, 2015

orlitzky commented May 19, 2015

orlitzky commented May 19, 2015

videlec commented May 19, 2015

vbraun commented May 19, 2015

videlec commented May 20, 2015

videlec commented May 20, 2015

videlec commented May 20, 2015

videlec commented May 20, 2015

vbraun commented May 20, 2015