Skip to content
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

Substitute with fold=true does not preserve rationals #1299

Open
hersle opened this issue Oct 7, 2024 · 4 comments · May be fixed by JuliaSymbolics/SymbolicUtils.jl#668
Open

Substitute with fold=true does not preserve rationals #1299

hersle opened this issue Oct 7, 2024 · 4 comments · May be fixed by JuliaSymbolics/SymbolicUtils.jl#668

Comments

@hersle
Copy link
Contributor

hersle commented Oct 7, 2024

It is annoying that

@variables x
substitute(1//2 * cos(x), x => 0)

gives 0.5 and not 1//2.

The reason is natural: cos(0) evaluates to 1.0 (not 1), giving (1//2) * 1.0, which is converted to 0.5.
Is there a way to fix this?

@ChrisRackauckas
Copy link
Member

@shashi is this intended?

@shashi
Copy link
Member

shashi commented Oct 25, 2024

well it's incidental like the OP said. Maybe fold could call a narrow_type function which turns floats into integers if is_integer is true.

@hersle
Copy link
Contributor Author

hersle commented Oct 29, 2024

How about if SymbolicUtils.simplify() by default uses the rule

using SymbolicUtils
r = @rule (~x::SymbolicUtils._isinteger) => Int(~x)

Then

r(7.0) # outputs 7

If you think this would work, can you point me to where I should add it for it to work on nested expressions? Like

@syms x
r(7.0*x) # should output 7x

I am not familiar enough with Symbolics to know how it should be added.

@ChrisRackauckas
Copy link
Member

That's a nice idea. It would probably go in https://github.com/JuliaSymbolics/SymbolicUtils.jl/blob/master/src/simplify_rules.jl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants