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

Literals as variables #17

Open
jpfairbanks opened this issue Sep 28, 2023 · 7 comments
Open

Literals as variables #17

jpfairbanks opened this issue Sep 28, 2023 · 7 comments
Assignees

Comments

@jpfairbanks
Copy link
Member

In the code that @GeorgeR227 presented today, it looked like there was var"1.4" = 1.4 in the generated output. We should probably just propagate the constant into the generated AST everywhere.

@lukem12345
Copy link
Member

lukem12345 commented Sep 28, 2023

Fair enough. I think it made some loop inside of compile_var easier to write initially. But this can be changed of course.

Part of the reason for why this change has not been high priority is that these literals are always tiny in terms of memory footprint. There is also some small amount of value extracted from having only lvalues in the main body of the function, although this is dubious.

I wonder whether this impacts anyone looking to re-implement compile. I don't imagine that any bugs will be introduced by mixing lvalues and rvalues inside that block. e.g. if someone is trying to use occurrences of variables as JSON keys or something.

@lukem12345 lukem12345 self-assigned this Sep 28, 2023
@GeorgeR227
Copy link
Contributor

Luckily the literals we're working with are primitives and so they should be pushed on the stack. This means no extra allocations and minimum access time.

When we do go to implement this, I imagine that we can just strip the var"..." encapsulation and just leave the contents in the gensim block. Calling eval then should just leave them as literals.

@jpfairbanks
Copy link
Member Author

Oh yeah we would then do 1.4 = 1.4 in the closure and then the hot loop would use 1.4 instead of var"1.4". My problem was mostly aesthetic, but it makes sense that it shouldn't affect performance the way it is written. Because the compile can constant prop them.

A benefit of writing a compiler that targets Julia is that a lot of the critical stuff in getting performance out of the compiled code is going get done by Julia when we eval our generated code.

@jpfairbanks
Copy link
Member Author

I looked into this and I see the problem. The variable names of a decapode are required to be symbols, not Julia Any. So that is an upstream problem of this.

@lukem12345
Copy link
Member

Ok. Is this now a DiagrammaticEquations issue regarding the acset type?

@jpfairbanks
Copy link
Member Author

Yeah, is there a way to mass migrate issues?

@lukem12345 lukem12345 transferred this issue from AlgebraicJulia/Decapodes.jl Jan 24, 2024
@jpfairbanks
Copy link
Member Author

@olynch, if we want to have literals in decapodes, then we should either relax the type constraint that variables have a variable name that is type symbol, or introduce a new table for literals. How relaxed can we go with a column type without breaking intertypes compatibility? For example, Union{Symbol, Number} should be fine. We can't do Union{Symbol, Float64} because with autodiff, the parameters be dual numbers, or for wave functions, they are complex.

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

No branches or pull requests

3 participants