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

Convert from PyStan back end to BridgeStan #3

Open
bob-carpenter opened this issue Mar 6, 2023 · 3 comments
Open

Convert from PyStan back end to BridgeStan #3

bob-carpenter opened this issue Mar 6, 2023 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@bob-carpenter
Copy link

It would be good for performance and for keeping up with Stan releases to replace PyStan with the BridgeStan interface to Stan. BridgeStan is also much simpler. We just released a stable 1.0 version with full doc and testing (please let us know if something's unclear). BridgeStan communicates through the low-level ctypes interface for Python, which lets us do everything in memory directly without copying. It's also good for compatibility and installation because it only requires memory compatibility, which is guaranteed by the C++ spec.

@abhiagwl
Copy link
Owner

abhiagwl commented Mar 7, 2023

Hey Bob! Thanks a lot for the suggestion. @justindomke pointed out BridgeStan is worth looking at (and potentially rewriting much of vistan to be compatible with BridgeStan.) I will update you once we settle on some ideas.

@abhiagwl abhiagwl self-assigned this Mar 7, 2023
@abhiagwl abhiagwl added the enhancement New feature or request label Mar 7, 2023
@justindomke
Copy link
Collaborator

justindomke commented Mar 7, 2023 via email

@bob-carpenter
Copy link
Author

In addition to interface.py, there is a line that's duplicated in utilities.py and interface.py to configure PyStan logging.

The PyStan model class and fit objects are used a lot, so those are going to have to be recoded for BridgeStan. I couldn't quite tell from casually looking at the code how the PyStan model and fit classes were used outside of interface.py if at all or if you just expose your own class Model. If it's the latter, it'll be a lot easier to retrofit BridgeStan.

It's super important to turn on Stan optimization. It's currently in this state:

        # THIS IS SLOW! TURN OPTIMIZATION ON SOMEDAY!
        extra_compile_args = ['-O1', '-w', '-Wno-deprecated']

Stan requires -O3 to be performant as we coded assuming extensive compiler optimizations in the Stan code base and the underlying matrix and math code (Eigen and Boost) do the same. -O3 should be on the order of 10x times -O0. I'm not sure about -O1. The nice thing is that the BridgeStan compiler will compile a model at -O3 a lot faster than PyStan will compile one at -O1. And BridgeStan just uses make directly for keeping everything up to date, so you could get rid of all the caching code.

I don't see where is_good_model could fail for a well-formed Stan model. Is this just to check that the Stan model isn't broken? (I can imagine you doing that because you used our old example-models, which were full of buggy models.). Running 100 iterations of sampling is really expensive to just check the model is well formed. 1 iteration should be enough if you absolutely have to do this for some reason.

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

No branches or pull requests

3 participants