-
-
Notifications
You must be signed in to change notification settings - Fork 81
Use cmdstan config and decouple stanfit objects from RunSet #851
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
Open
amas0
wants to merge
47
commits into
stan-dev:develop
Choose a base branch
from
amas0:use-save-cmdstan-config
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
d3fcc52
Add initial basic ConfigInfo model
amas0 81d08ab
Add flattening and parsing logic for config JSON
amas0 b3b20eb
Update flatten_value_dict to clarify variable naming
amas0 a5f8094
Add PathfinderConfig
amas0 d49c62c
Add LaplaceConfig
amas0 503a970
Add VariationalConfig
amas0 08c6d0e
Add GeneratedQuantitiesConfig
amas0 63ca9a5
Bump actions/cache from 4 to 5
dependabot[bot] 8f62f2e
Add first-pass CmdStanPathfinder rewrite
amas0 ca13a2e
Fixup Path conversion to string
amas0 c5a42db
Convert StanConfig to Generic for richer typing
amas0 610ad2f
Minor comment on config_file optionality
amas0 1e2f272
Refactor CmdStanLaplace
amas0 da04db4
Add default method_type for parse_config
amas0 8b1a39b
Refactor CmdStanMLE
amas0 3466024
Allow passing 'mode' directly to CmdStanLaplace.from_files
amas0 16eff3d
Refactor CmdStanVB to build from files
amas0 ef8cc74
Regenerate test output files for config.json compatibility
amas0 80f10d6
Remove config and metric files in sample test cleanup
amas0 f4b5c57
Update CmdStanMCMC to build from files
amas0 aa8ca89
Merge branch 'develop' into use-save-cmdstan-config
amas0 4f82501
Update CmdStanGQ to build from files
amas0 16a4e73
Source run configuration from config JSON, not CSV comments
amas0 78fc69f
Require config JSON in from_csv, drop CSV-comment fallback
amas0 8e529cc
Remove dead Stan CSV comment-parsing functions
amas0 3d7032d
Rename save_csvfiles and from_csv to save_output_files and from_outpu…
amas0 f1ec616
Bump minimum CmdStan version to 2.37
amas0 9c3b764
Name every output file explicitly when chains share a process
amas0 0833d8f
Simplify path handling and dispatch in from_output_files
amas0 fd60ea0
Order chains by id when reading output files
amas0 b411602
Read the shared run config when chains share a process
amas0 e62425e
Skip diagnostic and profile CSVs when reading a directory
amas0 8437240
Recover chain ids from the config when reading output files
amas0 0212413
Fix stanfit pickling with named run-config classes
amas0 ac9ddd2
Add shared StanFit base classes, port CmdStanPathfinder
amas0 bbcb0cf
Port CmdStanLaplace to the SingleFileFit base
amas0 8573f57
Port CmdStanVB to the SingleFileFit base
amas0 505623d
Port CmdStanMLE to the SingleFileFit base
amas0 eff860d
Keep draws() off fits whose rows are not all posterior draws
amas0 add7671
Add MultiChainFit base, port CmdStanGQ
amas0 28b1b3f
Port CmdStanMCMC to the MultiChainFit base
amas0 39aece0
Document inherited members of the stanfit classes
amas0 6d63994
Minor comment update for clarity
amas0 1f0e3d3
Take chain ids from filenames when reading output files
amas0 ced0bab
Simplify from_output_files edges and header-only CSV reads
amas0 98e52fa
Minor docs cleanup
amas0 e643667
Update CmdStanLaplace.from_file and tests
amas0 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. General comment: I really like how the changes look in this file, to me it really motivates the rest of the changes to be able to have this all be nicer! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's unfortunate we don't have a release in the 1.x cycle that formally deprecated from_csv, we might want to keep it around as an alias as a result. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My approach with this PR was to specifically target a 2.0 release, which is why I opted for a clean compatibility break here. That being said an alias that replicates the previous experience of using from_csv probably wouldn't be an issue?