Logcombine #1845

Merged
merged 5 commits into from Mar 3, 2013

Projects

None yet

3 participants

@smichr
Member
smichr commented Feb 27, 2013

No description provided.

@smichr smichr rewrite logcombine
The routine is much more targeted in terms of where the transformation
takes place and also combines logs that have a common coefficient, e.g.

x*log(3) + x*log(2) will now become x*log(6)

This only works if the coefficient is the same on both terms otherwise,
finding pairs to combine becomes costly.
3576531
@asmeurer asmeurer commented on an outdated diff Feb 28, 2013
sympy/simplify/tests/test_simplify.py
-@XFAIL
@asmeurer asmeurer and 1 other commented on an outdated diff Feb 28, 2013
sympy/simplify/tests/test_simplify.py
assert logcombine(Eq(y, -log(x)), force=True) == Eq(y, log(1/x))
+ assert logcombine(2*log(z)*log(w)*log(x) + log(z) + log(w)) == \
@asmeurer
asmeurer Feb 28, 2013 Member

Isn't this nondeterministic?

@smichr
smichr Feb 28, 2013 Member

No. ordered is used to determine the order in which the logs are folded up.

@asmeurer
asmeurer Feb 28, 2013 Member

I mean, maybe it's deterministic, but couldn't it equally come out to one of many things. It's seems wrong to test for specifically one of them.

@smichr
smichr Feb 28, 2013 Member

On Thu, Feb 28, 2013 at 10:38 AM, Aaron Meurer notifications@github.comwrote:

In sympy/simplify/tests/test_simplify.py:

 assert logcombine(Eq(y, -log(x)), force=True) == Eq(y, log(1/x))
  • assert logcombine(2_log(z)_log(w)*log(x) + log(z) + log(w)) == \

I mean, maybe it's deterministic, but couldn't it equally come out to one
of many things. It's seems wrong to test for specifically one of them.

What I mean is it's canonical -- it could be more than one way but in
SymPy on any platform in 32 or 64 bit this is how it will come out.

@asmeurer
asmeurer Mar 2, 2013 Member

Sure, it's always the same, but is it really correct to test for output that could potentially be different, say if the algorithm changes a little, or if the symbols were renamed? My point was, why not just test for logcombine(...) in [log(w**(2*log(z)*log(x)))..., log(x**(2*log(w)*log(z)))..., ...]?

Actually, this whole "putting logs as powers inside logs" thing has bothered me ever since I originally wrote this. The "right" think to do maybe requires a little more thought.

@asmeurer asmeurer and 1 other commented on an outdated diff Feb 28, 2013
sympy/simplify/simplify.py
@@ -3923,153 +3923,139 @@ def nsimplify_real(x):
return _real_to_rational(expr)
-def logcombine(expr, force=False):
+def logcombine(expr, force=False, first=True):
@asmeurer
asmeurer Feb 28, 2013 Member

What does first=True do?

@smichr
smichr Feb 28, 2013 Member

The first pass was used to handle the special expansion (which was cool) but is no longer needed. It's been removed.

@asmeurer asmeurer and 1 other commented on an outdated diff Feb 28, 2013
sympy/simplify/simplify.py
- else:
- args = []
- for i in expr.args:
- if i.func is log:
- args.append(_getlogargs(i))
- return flatten(args)
- return None
+ """
+ if first:
+ rv = bottom_up(expr, lambda x: logcombine(x, force))
+ else:
+ rv = expr
+
+ if not (rv.is_Add or rv.is_Mul):
+ # XXX this should not be here -- no other simplification works
+ # across the sides of Eq, do they?
@asmeurer
asmeurer Feb 28, 2013 Member

You can remove this if you want. I wrote this before I fully understood how Eq was used in SymPy.

@smichr
smichr Feb 28, 2013 Member

will do.

@smichr
Member
smichr commented Feb 28, 2013

Everything has been updated now.

@smichr
Member
smichr commented Feb 28, 2013

All lines but one (that I don't think can be hit but it's there for the case I can't think of right now) are covered.

@smichr smichr -log(x) -/-> log(1/x); check to put neg log's arg in denom
It's a matter of aesthetics ... I prefer to keep the -1 out in front of
the log. If there are others that it would join with, this doesn't matter
since log(x) - log(y) -> log(x) + log(1/y) -> log(x*1/y) -> log(x/y)
But when forming an exponent, log(-log(y)) looks better, I think, than
log(log(1/y)).
ce014a1
@jrioux
Member
jrioux commented Feb 28, 2013

SymPy Bot Summary: ✳️ Passed after merging smichr/logcombine (ce014a1) into master (5e0f9a3).
✳️ PyPy 2.0.0-beta-1; 2.7.3-final-42: pass
✳️ Python 2.7.2-final-0: pass
✳️ Python 3.2.1-final-0: pass
✳️ Sphinx 1.1.3: pass
Docs build command: make clean && make html-errors && make latex && cd _build/latex && xelatex sympy-*.tex

@asmeurer asmeurer commented on the diff Mar 2, 2013
sympy/solvers/ode.py
- /f(x)\ | / 1 1 \
- log|----| - | |- -- - -----------| d(u2) = 0
- \ C1 / | | u2 2 /1 \|
- | | u2 *sin|--||
- | \ \u2//
- |
- /
+ x
+ ----
+ f(x)
+ /
+ |
+ | / /1 \ \
+ | -|u2*sin|--| + 1|
+ | \ \u2/ /
+ log(f(x)) = log(C1) + | ----------------- d(u2)
@asmeurer
asmeurer Mar 2, 2013 Member

Shouldn't this have just reduced to C1?

@smichr
smichr Mar 2, 2013 Member

that's an ode simplify problem, I think.

@smichr
smichr Mar 2, 2013 Member

no -- simplify is False in this example

@asmeurer
asmeurer Mar 3, 2013 Member

Oh. That was probably because simplify hanged on this or something. What happens if simplify=False is removed?

@smichr
smichr Mar 3, 2013 Member

you get f(x) = C1*exp(...). It doesn't convey any new information so I left it in this more compact form.

@asmeurer
asmeurer Mar 3, 2013 Member

I see. Is that a feature or a bug of solve that it returns a partially solved result?

@smichr
smichr Mar 3, 2013 Member

from solve docstring

* when an object other than a Symbol is given as a symbol, it is
  isolated algebraically and an implicit solution may be obtained.
  This is mostly provided as a convenience to save one from replacing
  the object with a Symbol and solving for that Symbol. It will only
  work if the specified object can be replaced with a Symbol using the
  subs method.

So it's a feature.

@asmeurer asmeurer commented on the diff Mar 2, 2013
sympy/solvers/ode.py
@@ -2009,16 +2011,16 @@ def ode_1st_homogeneous_coeff_subs_dep_div_indep(eq, func, order, match):
\ x / \ x / dx
>>> pprint(dsolve(genform, f(x),
... hint='1st_homogeneous_coeff_subs_dep_div_indep_Integral'))
- f(x)
- ----
- x
+ f(x)
+ ----
+ x
+ /
+ |
+ | -h(u1)
+ log(x) = C1 + | ---------------- d(u1)
@asmeurer
asmeurer Mar 2, 2013 Member

It looks like it worked here.

@smichr
Member
smichr commented Mar 2, 2013

logs in logs

I would prefer not to fold logs together...and maybe just Rationals could become exponents?

@smichr
Member
smichr commented Mar 2, 2013

regarding the "logs as exponents"...this is a separate issue from this PR. Regarding the test. One reason to levae it in place is that someone may use logcombine on an expression and we should test that something canonical is obtained so that their test will also be canonical and won't fail at some point due to system variations. I can add a note that the test could appear otherwise but should be canonical.

@smichr
Member
smichr commented Mar 3, 2013

Committing in 24 if there are no further comments.

@asmeurer asmeurer commented on the diff Mar 3, 2013
sympy/solvers/ode.py
@@ -1942,13 +1944,13 @@ def ode_1st_homogeneous_coeff_best(eq, func, order, match):
>>> f = Function('f')
>>> pprint(dsolve(2*x*f(x) + (x**2 + f(x)**2)*f(x).diff(x), f(x),
... hint='1st_homogeneous_coeff_best', simplify=False))
@asmeurer
asmeurer Mar 3, 2013 Member

And the same here.

@smichr
smichr Mar 3, 2013 Member

That one looks better with simplify=True

                    /      ___________\
                    |     /     2     |
                    |    /   3*x      |
log(f(x)) = C1 - log|   /   ----- + 1 |
                    |3 /     2        |
                    \\/     f (x)     /
@smichr
smichr Mar 3, 2013 Member

It's a line shorter, but a couple of columns wider -- do you have a preference?

@asmeurer
asmeurer Mar 5, 2013 Member

Unless there's a performance issue, use simplify=True. The flag is only there as an advanced work-around feature, so doesn't really belong in this example. And I prefer to have C1 over log(C1).

@asmeurer
asmeurer Mar 5, 2013 Member

That is to say, don't call the simplify flag at all.

@asmeurer
Member
asmeurer commented Mar 3, 2013

+1 to merging. But look into the ODE thing.

@asmeurer
Member
asmeurer commented Mar 3, 2013

Regarding combining logs, an argument for always combining no matter how silly it can get: suppose you have an expression and you want to simplify exp(expression). Then you might apply logcombine to it. In that case, you want all logarithms to reduce to one, even if it means having powers (because for some reason, you would prefer a**log(x) over exp(log(a)*log(x))). At any rate, I'll have to think about what sort of situations can even lead to the product of logarithms.

Actually, this leads to an interesting question: when does x**log(y) == y**log(x)? They look different, but I claim that at least for x, y > 1, they are the same, because then they are both positive, and applying log to each one gives log(x)*log(y), and log is one-to-one (at least for positive arguments).

@asmeurer
Member
asmeurer commented Mar 3, 2013

I got curious about this, so I worked most of it out at http://asmeurersympy.wordpress.com/2013/03/03/when-does-xlogy-ylogx/. This has additionally began to answer one of my long time questions, which is to give exact conditions for the log identities to hold (I believe we have log(x**a) == a*log(x) iff arg(x**a) == a*arg(x), though I need to think about this a little longer to be completely sure. I also need to think how to describe this in a more useful way).

@jrioux
Member
jrioux commented Mar 3, 2013

SymPy Bot Summary: ✳️ Passed after merging smichr/logcombine (3d231ca) into master (009496a).
✳️ PyPy 2.0.0-beta-1; 2.7.3-final-42: pass
✳️ Python 2.7.2-final-0: pass
✳️ Python 3.2.1-final-0: pass
✳️ Sphinx 1.1.3: pass
Docs build command: make clean && make html-errors && make latex && cd _build/latex && xelatex sympy-*.tex

@smichr
Member
smichr commented Mar 3, 2013

Regarding the conditions for when the equality applies. We can say they apply whenever log can be applied to both sides so that log(x)*log(y) = log(y)*log(x) And log(ab) = b*log(a) when b (the upper log in the expression of interest) is real and a (the base) is positive. This matches with our x,y>0 since then the base is positive and the log terms are real. The other 3 cases (x=y or one arg = e or one arg = 1) are special cases where the expression reduces to a less complicated expressions, either xlog(x) == x**log(x) or x = x or 1 == 1. It's a nice exercise. If solve were updated to give these solutions, I suppose it should give [{x: y}, {x: E}, {y: E}, {x: 1}, {y: 1}]. Perhaps you can open an issue for this.

@smichr
Member
smichr commented Mar 3, 2013

I'll commit this as is.

@smichr smichr merged commit e523a10 into sympy:master Mar 3, 2013

1 check passed

default The Travis build passed
Details
@smichr smichr deleted the smichr:logcombine branch Mar 3, 2013
@asmeurer
Member
asmeurer commented Mar 6, 2013

SymPy Bot Summary: Could not fetch the branch smichr/logcombine.
@smichr: Please make sure that smichr/logcombine has been pushed to GitHub and run the sympy-bot tests again.

@asmeurer
Member

I just remembered that the example at the end of the docstring of simplify shows exactly how to avoid powers from logs. I have no idea why it works, though (in particular, why simplify gives the secondary result), so I don't know if it will automatically generalize. But certainly it could if we added a flag to logcombine to avoid pulling in complicated powers, and tried both True and False for it in simplify().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment