probcomp / Venturecxx

Primary implementation of the Venture probabilistic programming system
http://probcomp.csail.mit.edu/venture/
GNU General Public License v3.0
28 stars 6 forks source link

Simplify the Args situation #520

Open axch opened 8 years ago

axch commented 8 years ago

The Venture source currently contains seven (!) Python classes whose name ends in the token Args:

One of the causes for this proliferation of Argh is that officially, the SP interface is supposed to permit SPs to obtain all sorts of information about their evaluation context from the interpreter:

Most SPs, however, only care about the actual arguments and the prng, with a reasonable minority that also cares about the spaux and madeSPAux. The other stuff is both rarely needed and not always (readily) available, hence the alternatives.

The thing to do seems to be to

A somewhat higher-achiever intervention would consist of separating "normal" SPs from "magic" ones (what John Shutt called "operators" when specifying Kernel), and not even passing all the bizarre contextual information to "normal" SPs. Perhaps also disallowing "magic" SPs from being added to the system without patching the source. Besides simplifying the interface to "normal" SPs, this would have the added benefit that a future compiler can treat the set of "magic" SPs as closed and reason about each in depth, without having to be concerned about maintaining the ability to meet the "magic" interface for arbitrary user-addable SPs.

Thoughts?

riastradh-probcomp commented 8 years ago

How about 'Invocation' or something instead of 'Args'? 'Args' suggest it contains arguments, but it does much more than that.

Separating magic and normal SPs sounds good to me. I don't like magic!

axch commented 8 years ago

If we're going to rename the thing, I was thinking "Context" or "CallContext" or something. I balked at proposing that right away, because all the local variable names for it would need to be changed too.

lenaqr commented 8 years ago

In my Mite branch I have already elected to use the name "Context" for a different object... https://github.com/probcomp/Venturecxx/blob/mite-rev3/backend/mite/evaluator.py#L27

not that this isn't amenable to change.

axch commented 8 years ago

Progress update:

Justification for the remaining Args classes:

Outstanding desiderata: