doc for `fixed_param` should mention it's not necessary
CmdStanPy automatically resolves whether it needs fixed_param = True or not depending on the number of parameters in the model. This isn't clear in the documentation, which makes it sound like this is required. This caused someone to open an issue against my tutorial because they read the CmdStanPy doc:
https://github.com/bob-carpenter/stan-getting-started/issues/6
My response there shows that you don't need fixed_param = True.
On the most recent couple versions of stan, fixed_param is actually never necessary, the behavior where it would use it with 0 parameter models is purely legacy at this point.
I think it would make sense to remove fixed_param as an argument to the sample function. If people wanted that behavior for whatever other reason, we could add a separate function for it, but it currently really tortures the sample logic and return value to support both.
I'd be even happier getting rid of it. Can we just ignore it for some limited backward compatibility rather than throwing errors? That will let programs that previously worked with fixed_param = True to still work (cases where it'd throw an error just get ignored).
A program that was previously manually specifying fixed_param=True would probably be at least as upset if we started transparently returning normal draws from HMC as it would at us erroring. Anyway, I was thinking of this primarily for the 2.0 release we've been circling in discussions for a bit now.
It sounds instead like you'd just like to throw an error whenever fixed_param shows up in a new program? That's going to break existing programs. So I see why you say 2.0.
There are two relevant cases:
-
A client specifies
fixed_param=Truefor a Stan program with no parameters and sampling currently works as intended to generate transformed parameters and/or generated quantities. If we throw an error when we seefixed_param, their working code will break. -
A client specifies
fixed_param=Truefor a Stan program with a parameter vector of size greater than zero. In this case, their program will fail in the existing setup (I hope). If we throw an error when we seefixed_paramit will still be broken. But if we just go ahead and HMC sample, I think it does the "right" thing in "fixing" the client's broken code. But the optics are weird because their code saysfixed_param=Truebut actually runs sampling.
Can we just generate a warning? Or do you really think it's better just to break user code in case (1)? I guess a more sophisticated fix would be to detect when you see fixed_param=True and if there are parameters, throw an error and if there aren't just ignore it.
A client specifies fixed_param=True for a Stan program with a parameter vector of size greater than zero. In this case, their program will fail in the existing setup (I hope)
This is not true. You can use fixed param as a sampler for any number of parameters, it will just use the initial values every iteration. I'm not sure why this would be used, but someone certainly could do it.
If we're discussing doing this as part of an api-breaking-version anyway, I do think it's better to actually make the breaking change in order to remove that consideration (and associated tests, doc, etc) from the code. A warning would be good as a heads-up before said version
Good point and now I get your earlier comments. I keep forgetting about that "feature" as it was never an intended use, just a side-effect of a missed error check. At least I've never heard of a use for it.
I don't have a complete grasp of where fixed_param comes into play in the library. My understanding is that it comes up in two areas: if the user specifies it via an argument to sample() or if the model has no parameters (and they are using CmdStan < 2.36). From this discussion it sounds like we can possibly remove the user specified option, is that correct? Would that mean that eventually if we drop support for CmdStan < 2.36, we could remove the logic that handles the fixed param cases entirely?
As it stands, fixed_param is a pretty annoying edge case that needs to handled all over the place, so being able to do away with it in some way would save some headaches.