Skip to content

Comments

Allocations#310

Open
jverzani wants to merge 6 commits intosymengine:masterfrom
jverzani:allocations
Open

Allocations#310
jverzani wants to merge 6 commits intosymengine:masterfrom
jverzani:allocations

Conversation

@jverzani
Copy link
Contributor

@jverzani jverzani commented Jan 28, 2026

A few small adjustments

  • added const to several constant variables

  • allow TermInterface.operation to work with SymFunction objects

  • add nameof method for SymFunction objects

  • Conversion of :ℯ (\euler) (Fixes bug)

  • add unwrap_const to mirror SymbolicUtils behaviour (no error)

  • add iseven and isodd; add tests (allocate due to conversion to Integer)

  • adjust behavior of _convert for expressions (Really want to avoid this, but somehow writing convert(Expr, x) and getting a symbol seems wrong. Add test for behaviour.

src/mathfuns.jl Outdated
Comment on lines 192 to 193
Base.Symbol(s::SymFunction) = Symbol(s.name)
Base.nameof(s::SymFunction) = Symbol(s)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these be flipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got cute? They could both resolve to Symbol(s.name). (I was looking for a generic that would give the symbol and not the general symbol for SymFunction and wasn't sure which should be preferred for generic programming.)

@jverzani
Copy link
Contributor Author

Is this good to go? I'm happy to merge but might need your help again with juliaregistrator.

src/mathops.jl Outdated

## Logical operators
Base.:<(x::SymbolicType, y::SymbolicType) = N(x) < N(y)
Base.:<(x::SymbolicType, y::SymbolicType) = is_constant(x) && is_constant(y) && N(x) < N(y)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x < y where x, y are Symbols would now give False silently?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the idea, as the comparison isn't true. But I think I have this incorrect. There is now:

  • isless -- Non-numeric types with a total order should implement this function (then < would fall back to isless. (But is there a natural total order?)
  • < -- New types with a canonical partial order should implement this function for two arguments of the new type.
  • isunordered -- Return true if x is a value that is not orderable according to <,

We have defined isless and < but it seems in a manner that is not quite right. (isless and < had the same redundant definition.)

I'll roll this back and open an issue so it can be through through.

src/numerics.jl Outdated


## julia predicates we can mirror
Base.iseven(x::Basic) = is_a_Integer(x) && iseven(convert(Integer, x))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should implement it for BasicType{:Integer} only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good question. For floating point values iseven is defined and it checks for the value being an integer (2.0 is considered an integer) and then checks. Perhaps it should be a test with is_constant: Base.iseven(x::Basic) = is_constant(x) && iseven(N(x)) which is more general and doesn't seem to allocate any more

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even then for a symbolic x we say it is not even, when we should be erroring out for symbolic values that are not integers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, let's be consistent. (I just rolled back the < defintion and will roll back this one)

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 this pull request may close these issues.

2 participants