search

stan-dev / cmdstanpy

CmdStanPy is a lightweight interface to Stan for Python users which provides the necessary objects and functions to compile a Stan program and fit the model to data using CmdStan.
BSD 3-Clause "New" or "Revised" License
151 stars 69 forks source link

Scopes of Runset are inconsistent among stanfit classes #719

Closed lucidfrontier45 closed 10 months ago

lucidfrontier45 commented 10 months ago

Summary:

Scopes of Runset are inconsistent among stanfit classes

Description:

CmdStanGQ, CmdStanMCMC, CmdStanMLE and CmdStanVB hold a Runset as self.runset. CmdStanLaplace and CmdStanPathfinder hold it as self._runset.

Are there any intention that Runset is private in Laplace and Pathfinder? Can I submit a Pull Request to change all of them to public self.runset?

Current Version:

1.2.0

WardBrian commented 10 months ago

Can I submit a Pull Request to change all of them to public self.runset?

Sure, consistency seems good. Can I ask what you are using the RunSet for? A lot of the details of that class are kind of messy and more internal APIs

lucidfrontier45 commented 10 months ago

I was planning to create a class that can wrap any Stan model. I was implementing a serialize/deserialize function that can work on any stan_fit classes i.e. MLE, VB, MCMC, Laplace, Pathfinder. To do so I tried to get csv files from runset.csv_files and zip them. Then I found runset field is not public in Laplace and Pathfinder.

I also noticed this issue. https://github.com/stan-dev/cmdstanpy/issues/705 Generated quantity does not support Laplace and Pathfinder fit result currently. Is it the reason that runset is private in Laplace and Pathfinder? If so, I think I can first focus on MLE/MCMC/VB and keep runset of Laplace/Pathfinder private.

WardBrian commented 10 months ago

I think it not being public currently is unrelated. Generate quantities just isn't that interesting for those algorithms

lucidfrontier45 commented 10 months ago

https://github.com/stan-dev/cmdstanpy/blob/535c4e6d666f514b0860c49545ae3cbada86168f/cmdstanpy/model.py#L1280

As you can see the generate_quantities is accessing previous_fit.runset. I expect when generate_quantities for Laplace and Pathfinder is implemented, the runset field of those two fit classes will also be public. I close this issue and wait until then.