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

Core name and syntax changes #42

Open
marcoct opened this issue Jan 10, 2019 · 7 comments
Open

Core name and syntax changes #42

marcoct opened this issue Jan 10, 2019 · 7 comments
Labels

Comments

@marcoct
Copy link
Collaborator

marcoct commented Jan 10, 2019

Based on a discussion with @vkmvkmvkmvkm:

  • change Assignment to Trie.

  • change get_assmt to get_choices. It will return a Trie.

  • change Distribution to UnboxedGenerativeFunction (or something similar)

Also see probcomp/GenExperimental.jl#19

@marcoct
Copy link
Collaborator Author

marcoct commented Jan 24, 2019

Other name changes:

  • consider changing @addr(<call>, <addr>) to @trace(<call>, <addr>), and change @splice(<call>) to @trace(<call>). This resolves the suggestion in Probcomp stack integration GenExperimental.jl#48, and also makes more sense regarding automatic differentiation.

  • consider changing Gen.initialize to Gen.trace

  • consider changing Gen.backprop_params to Gen.backprop_params!. This makes it more explicit that some state associated with the generative function (the gradient accumulators for trainable parameters) is being mutated.

@fsaad
Copy link
Collaborator

fsaad commented Jan 24, 2019

consider changing Gen.initialize to Gen.trace

I think Gen.trace can be confusing if we are going to use it as both a noun ad a verb. In Gen we commonly have the trace data structure lying around (which is a noun, representing the execution trace), as well as, according the proposal, the verb Gen.trace which is telling Gen to trace the function and then return the execution trace. Also namespace collisions will arise if Gen has a trace function.

Maybe Gen.get_trace?

@marcoct
Copy link
Collaborator Author

marcoct commented Jan 24, 2019

Other options:

  • Gen.generate_trace.
  • Gen.traced_exec

@marcoct marcoct changed the title Name changes Name and syntax changes Feb 1, 2019
@marcoct marcoct changed the title Name and syntax changes Core name and syntax changes Feb 1, 2019
@marcoct
Copy link
Collaborator Author

marcoct commented Feb 3, 2019

I thought about renaming the initialize method in the Gen function interface.

The proposed new name is generate_trace. This has the word "trace" in it. It also has the word "generate", which is appropriate since a generative function is precisely one that has this method.

    (trace, weight) = generate_trace(gen_fn, args)

Return a trace of the generative function.

    (trace, weight) = generate_trace(gen_fn, args, constraints)

Return a trace of the generative function that is consistent with the given constraints on the random choices.

The name initialize is not ideal, because "initializing" assumes the trace is going to be changed later, which is not always the case, so the current name only makes sense for certain usages that update the trace. it also doesn't have the word "trace" in it, which is the essential difference between the behavior here and the behavior of just calling the generative function in regular Julia usage, e.g. gen_fn(args...).

I also considered but these imply that the generating process is forward simulation, which isn't always the case, and also describe the method from an internal perspective, not from what the user wants to do with the function (like generate_trace):

  • traced_apply
  • traced_exec
  • traced_call

get_trace makes it sound lke the trace already exists and this is a "getter" method (like all the other usages of get_X in Gen).

@marcoct
Copy link
Collaborator Author

marcoct commented Feb 9, 2019

These changes were made in probcomp/GenExperimental.jl#67

@marcoct marcoct closed this as completed Feb 9, 2019
@marcoct
Copy link
Collaborator Author

marcoct commented Feb 9, 2019

Actually, except for renaming distributions to a type of generative function. That has not been changed yet. Reopening.

@marcoct marcoct reopened this Feb 9, 2019
@georgematheos
Copy link
Contributor

@marcoct is this closed by #274 / #282 ?

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

No branches or pull requests

3 participants