Skip to content

Feature/22 better error handling#280

Merged
mitzimorris merged 6 commits intodevelopfrom
feature/22-better-error-handling
Aug 14, 2020
Merged

Feature/22 better error handling#280
mitzimorris merged 6 commits intodevelopfrom
feature/22-better-error-handling

Conversation

@mitzimorris
Copy link
Copy Markdown
Member

Submission Checklist

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

Summary

When CmdStan method returns a non-zero error code, return as much useful information as can be scraped out of the stdout and stderr messages, plus information about the run and locations of resulting output files, if any.

Exposed and expanded RunSet method get_err_msgs; this function was written a long time ago, but wasn't being called by the CmdStanModel methods. It attempts to get just the relevant messages from the information spew that's written to stdout, in addtion to all msgs written to stderr. It seems to do an OK job on the most common and annoying problem - return code 70 which happens when there's a mismatch between the model's data block variable declarations and the input data.

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 Aug 14, 2020

Codecov Report

Merging #280 into develop will increase coverage by 0.13%.
The diff coverage is 81.81%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #280      +/-   ##
===========================================
+ Coverage    75.89%   76.03%   +0.13%     
===========================================
  Files            9        9              
  Lines         2199     2195       -4     
===========================================
  Hits          1669     1669              
+ Misses         530      526       -4     
Impacted Files Coverage Δ
cmdstanpy/model.py 81.69% <57.14%> (+0.71%) ⬆️
cmdstanpy/stanfit.py 95.37% <93.33%> (-0.12%) ⬇️

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 d758170...c6d0c57. Read the comment docs.

Copy link
Copy Markdown
Contributor

@ahartikainen ahartikainen left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread cmdstanpy/model.py
inits=_inits,
output_dir=output_dir,
save_diagnostics=save_diagnostics,
save_diagnostics=False,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not needed anymore?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

right - at the stan::services layer, https://github.com/stan-dev/stan/tree/develop/src/stan/services/optimize - if you look at these methods, they don't take a diagnostics file as an argument.

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.

3 participants