TuringLang / DynamicPPL.jl

Implementation of domain-specific language (DSL) for dynamic probabilistic programming
https://turinglang.org/DynamicPPL.jl/
MIT License
164 stars 29 forks source link

Harmonise our use of !! #653

Open mhauru opened 2 months ago

mhauru commented 2 months ago

We are currently inconsistent in our use of methods with !! at the end of the name. We sometimes call them but discard the return value, assuming they mutate; some methods which return their value are only marked width !; there may be a few that have !! in the name but don't in fact always return a value; etc. Once #555 is done I would like to take a pass over the codebase and make it such that

I would also like to switch to using !! methods in many places where we currently use ! methods. For instance, in #555 I'm building towards VarInfo being such that you can push!! new values to it that don't fit the element or VarName type of the current VarInfo, and if need be a new object is returned with expanded types, although most often that's not necessary and the old object is returned mutated. This might help simplify building VarInfos that with concrete types.

devmotion commented 2 months ago

! methods never return a value.

It's quite common in Julia that ! functions return the argument that is mutated since this makes it easier to compose functions.

The other two points though are a requirement of the bang-bang convention: Since they may mutate but don't have to, they must return the result and one must not assume mutation.

mhauru commented 2 months ago

It's quite common in Julia that ! functions return the argument that is mutated since this makes it easier to compose functions.

Fair point. I first thought, if that's the case, why not call it a !! function. But I guess sometimes you may want a guarantee that the returned object isn't a new one. Happy to strike the "! methods never return a value" request of the list, the other two are the important ones.

torfjelde commented 1 month ago

Very much in favour of this :+1: