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

User-configurable gensym #178

Open
cscherrer opened this issue Aug 31, 2020 · 3 comments
Open

User-configurable gensym #178

cscherrer opened this issue Aug 31, 2020 · 3 comments

Comments

@cscherrer
Copy link
Owner

What if I have another variable named _y_dist?

Great point. At one point, Soss used to use gensym for new symbols. But for looking at the source code, this can be pretty confusing. So currently I'm using a leading underscore for "internal" variables. For example.

julia> m = @model begin
           x ~ Normal()
       end;

julia> Soss.sourceLogpdf(m)
quote
    _ℓ = 0.0
    _ℓ += logpdf(Normal(), x)
    return _ℓ
end

Maybe a better way to do this would be to have a function makename (or something) that's user-configurable with some easy default. So currently it would be

makename(:ℓ) == :_ℓ

@DilumAluthge what's a sensible way to set this up so users can change it? Maybe as a Ref{Function}?

Originally posted by @cscherrer in #176 (comment)

@DilumAluthge
Copy link
Collaborator

Something simple like this should be sufficient:

const internal_name_prefix = Ref{Symbol}(:_)

function make_internal_name(original_name::Symbol)
    return Symbol(internal_name_prefix[], original_name)
end

Example usage:

julia> make_internal_name(:foo)
:_foo

julia> make_internal_name(:bar)
:_bar

julia> internal_name_prefix[] = :_internal_
:_internal_

julia> make_internal_name(:foo)
:_internal_foo

julia> make_internal_name(:bar)
:_internal_bar

@cscherrer
Copy link
Owner Author

Thanks @DilumAluthge! Just to check, this does disallow gensyming the names. Do you think that's ok? OTOH one problem with gensym is that you need to get all the variables you'll need ahead of time - otherwise calling it twice in the codegen would give different results.

Final concern is that we'll need this a lot, and make_internal_name is a lot to type. Maybe something like soss_name, soss_id, or soss_var, or one of those without the underscore? Or something similarly short?

@DilumAluthge
Copy link
Collaborator

Yeah I think it's fine to disallow gensyms. They make the code harder to read anyway.

soss_id seems good to me!

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

2 participants