Skip to content

Simplify NaNMath.pow substitution#689

Open
devmotion wants to merge 1 commit intoJuliaSymbolics:masterfrom
devmotion:dw/pow_simplify
Open

Simplify NaNMath.pow substitution#689
devmotion wants to merge 1 commit intoJuliaSymbolics:masterfrom
devmotion:dw/pow_simplify

Conversation

@devmotion
Copy link
Contributor

@devmotion devmotion commented Jan 9, 2025

Line 149 was wrong, but it seems it was not caught by the tests since this simplification is applied already when the expression is constructed. Which made me wonder whether we could remove this branch completely from the NaNMath.pow substitution.

@devmotion devmotion mentioned this pull request Jan 13, 2025
Merged
@AayushSabharwal
Copy link
Member

AayushSabharwal commented Jan 13, 2025

Does this not lose the inv transformation?

@devmotion
Copy link
Contributor Author

devmotion commented Jan 13, 2025

Initially I assumed it would, but based on the tests I ran it didn't, which is why I opted for removing this special branch completely. My hypothesis was that it is already covered by the existing rewriting/simplification performed for ^ at construction of the symbolic expression, as mentioned in https://docs.sciml.ai/SymbolicUtils/stable/#Simplification.

@AayushSabharwal
Copy link
Member

AayushSabharwal commented Jan 13, 2025

The NaNMath.pow case hits this, since while a ^ (-2) might transform into 1 / a ^ 2, NaNMath.pow(a, -2) transforms into a ^ (-2) which needs the rewrite rule.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@devmotion @AayushSabharwal