Skip to content

Project/release 1.0 281 optimize#450

Merged
mitzimorris merged 6 commits intodevelopfrom
project/release-1.0-281-optimize
Sep 7, 2021
Merged

Project/release 1.0 281 optimize#450
mitzimorris merged 6 commits intodevelopfrom
project/release-1.0-281-optimize

Conversation

@mitzimorris
Copy link
Copy Markdown
Member

Submission Checklist

  • Run unit tests
  • Declare copyright holder and open-source license: see below

Summary

Addresses issue #281 - the call to the optimize method now has argument save_iterations, default is False. When set to True, will set CmdStan command line arg save_iterations=1, which causes the optimizer to write intermediate iterations to the Stan CSV output file.

The intermediate iterations are available either as a numpy.ndarray object over all data columns in the Stan CSV file, or as a pandas.DataFrame. The stan_variable methods don't expose the intermediate iterations. The reasoning here is that intermediate iterations are only of interest when trying to debug a model during development, or to debug an algorithm - it's not a very common use and doesn't really warrent adding more logic to stan_variable. That said, it wouldn't be that hard to extend this functionality to stan_variable - can be added now or later.

Also brings CmdStanPy into line with CmdStanR w/r/t allowing user to retrieve estimates even is optimizer fails to converge - stan-dev/cmdstanr#314 by adding argument require_converged - when False, use can retrieve MLE estimates (same as behavoir for method variational). If the optimizer fails to converge, the resulting CmdStanMLE object will return a warning when the user accesses the estimates.

Changes to the code base:

  • cmdstan_args.py - add argument save_iterations to OptimizeArgs
  • model.py - add logic to handle add'l args and to handle failure to converge
  • stanfit.py, class CmdStanMLE - logic to capture whether or not optimizer converged, and new methods to return all iterations.
  • utils.py - changed parsing logic for parsing Stan CSV files output by optimizer.

Copyright and Licensing

Please list the copyright holder for the work you are submitting (this will be you or your assignee, such as a university or company): Columbia University

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the following licenses:

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 3, 2021

Codecov Report

Merging #450 (1d8a65f) into develop (3348211) will decrease coverage by 0.59%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #450      +/-   ##
===========================================
- Coverage    76.96%   76.36%   -0.60%     
===========================================
  Files           27       27              
  Lines         8664     8979     +315     
===========================================
+ Hits          6668     6857     +189     
- Misses        1996     2122     +126     
Impacted Files Coverage Δ
a/cmdstanpy/cmdstanpy/cmdstanpy/utils.py 72.22% <0.00%> (-3.05%) ⬇️
cmdstanpy/cmdstanpy/utils.py 68.94% <0.00%> (-2.79%) ⬇️
...runner/work/cmdstanpy/cmdstanpy/cmdstanpy/utils.py 68.94% <0.00%> (-2.79%) ⬇️
cmdstanpy/cmdstanpy/stanfit.py 91.63% <0.00%> (-0.66%) ⬇️
...nner/work/cmdstanpy/cmdstanpy/cmdstanpy/stanfit.py 91.63% <0.00%> (-0.66%) ⬇️
a/cmdstanpy/cmdstanpy/cmdstanpy/stanfit.py 91.42% <0.00%> (-0.64%) ⬇️
cmdstanpy/cmdstanpy/__init__.py 86.36% <0.00%> (ø)
a/cmdstanpy/cmdstanpy/cmdstanpy/__init__.py 86.36% <0.00%> (ø)
...ner/work/cmdstanpy/cmdstanpy/cmdstanpy/__init__.py 86.36% <0.00%> (ø)
cmdstanpy/cmdstanpy/cmdstan_args.py 95.23% <0.00%> (+0.03%) ⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3348211...1d8a65f. Read the comment docs.

@WardBrian
Copy link
Copy Markdown
Member

Don't think I can review this until Tuesday, but it will be the first thing I do then! Right off the bat, I think stan_variables should feature a flag to include these like we do with inc_sample in GQ and include_warmup in MCMC, no?

Comment thread cmdstanpy/stanfit.py
Comment thread cmdstanpy/utils.py Outdated
Copy link
Copy Markdown
Member

@WardBrian WardBrian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mitzimorris mitzimorris merged commit 3224542 into develop Sep 7, 2021
@mitzimorris mitzimorris deleted the project/release-1.0-281-optimize branch September 17, 2021 19:15
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.

expose argument save_iterations on optimize method; return all iterations

4 participants