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

update pathfinder branch #727

Merged
merged 594 commits into from
Jul 31, 2023
Merged

update pathfinder branch #727

merged 594 commits into from
Jul 31, 2023

Conversation

rok-cesnovar
Copy link
Member

@rok-cesnovar rok-cesnovar commented Dec 3, 2022

@jgabry wrote in #726:

[Don't merge this yet]

Updating the pathfinder branch to incorporate latest changes to master.

@SteveBronder there aren't any conflicts, but before I merge this I want to check with you to make sure this won't harm anything in your pathfinder_testing repo. It would be good to keep that up to date with master if possible (e.g., so Chris from BLS and anyone else testing pathfinder can use latest cmdstanr features and bug fixes and also to avoid merge conflicts in the future).

@jgabry jgabry temporarily deployed to github-pages July 27, 2023 14:59 — with GitHub Pages Inactive
@jgabry jgabry temporarily deployed to github-pages July 27, 2023 22:26 — with GitHub Pages Inactive
@jgabry jgabry temporarily deployed to github-pages July 27, 2023 23:27 — with GitHub Pages Inactive
@jgabry jgabry temporarily deployed to github-pages July 28, 2023 00:00 — with GitHub Pages Inactive
@jgabry jgabry temporarily deployed to github-pages July 30, 2023 15:55 — with GitHub Pages Inactive
@andrjohns andrjohns temporarily deployed to github-pages July 31, 2023 14:45 — with GitHub Pages Inactive
@SteveBronder
Copy link
Collaborator

Should this be closed and we can open up a new PR for pathfinder in cmdstanr?

@rok-cesnovar
Copy link
Member Author

I think we should merge this, which would merge master into the pathfinder branch and then review, test and get it merged?

@mitzimorris
Copy link
Member

how does the user call the Pathfinder method? @WardBrian @jgabry - the usual concern - making the interfaces line up.

@jgabry
Copy link
Member

jgabry commented Jul 31, 2023

Agree with @rok-cesnovar that we should merge this (this PR doesn't merge pathfinder into master, just updates the pathfinder branch). Then we can have a discussion with CmdStanPy devs about what pathfinder should look like in CmdStanR/Py and either start a new pathfinder branch or update this one (if the design is similar enough to what's here already). That make sense to everyone?

@jgabry
Copy link
Member

jgabry commented Jul 31, 2023

Will go ahead and merge this (which just updates the pathfinder branch) and we'll eventually do a new PR (like @SteveBronder mentioned) to get pathfinder into the master branch when the design is agreed upon for all the interfaces.

@jgabry jgabry merged commit 6be1c02 into feature/pathfinder Jul 31, 2023
15 checks passed
@SteveBronder
Copy link
Collaborator

Agree! I wrote it a while ago, and very quickly, so we might be able to save some things from this branch but may also just need to get rid of it.

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

Successfully merging this pull request may close these issues.

10 participants