-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Adding Min/Max support for decompogen.py #13623
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
Conversation
Now decompogen() function can support Min/Max functions
@smichr, please review |
ping @aktech , can you take a look at this? It seems too fragile to only be traversing certain args. Perhaps you could make a better suggestion. |
@@ -47,6 +50,14 @@ def decompogen(f, symbol): | |||
result += [f.subs(f.args[0], symbol)] + decompogen(f.args[0], symbol) | |||
return result | |||
|
|||
# ===== Max/Min Functions ===== # | |||
if isinstance(f, (Max, Min)): | |||
result += [type(f)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding what you are going to traverse in the for
loop makes this fragile -- trust recursion here:
if symbol not in f.free_symbols or f == symbol:
return [f]
if isinstance(f, (Function, Pow)):
...
if isinstance(f, (Max, Min)):
rv = [type(f)]
for i in f.args:
rv.extend(decompogen(i, symbol))
return rv
But I don't know what is going to be done with the results and whether it is important to know that Min and Max were present...so maybe it should start out as rv = []
.
@prajwalgatti Are you still interested to work on it? |
Fixes #13612
Now the code produces the following output:
>>>decompogen(Max(x, 3), x)
[Max]
>>>decompogen(Max(Min(sin(tan(x**4)), pi), sin(tan(x))), x)
[Max, sin(x), tan(x), Min, sin(x), tan(x), x**4]