Skip to content

Fix diagnose: send error= argument and remove its output directory - #873

Open
atarutin wants to merge 1 commit into
stan-dev:developfrom
atarutin:fix/871-diagnose-error-output-dir
Open

atarutin wants to merge 1 commit into
stan-dev:developfrom
atarutin:fix/871-diagnose-error-output-dir

Conversation

@atarutin

Copy link
Copy Markdown
Contributor

Submission Checklist

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

Summary

Closes #871.

CmdStanModel.diagnose had two defects.

  1. The error argument was sent to CmdStan as epsilon=<error>, so CmdStan never received error=, and passing both arguments put epsilon on the command line twice. It now sends error=<error>.
  2. The output directory was created with tempfile.mkdtemp(prefix=self.name, dir=_TMPDIR) and never removed. It is now a tempfile.TemporaryDirectory(prefix=self.name, dir=_TMPDIR) in the same with statement as the JSON inputs, as in Remove log_prob's temporary output directory when the call returns #869. It is still nested under _TMPDIR. The output is read inside the with block, so the returned DataFrame is unchanged.

Tests (test/test_model.py):

  • test_diagnose_error_argument checks that the logged command carries error= and a single epsilon=.
  • test_diagnose_removes_output_dir calls diagnose three times on the success path and on the RuntimeError path, and asserts that the entries of _TMPDIR are unchanged and that the output was written under _TMPDIR.

Testing

Python 3.12, pip install -e ".[all,test]":

  • pytest test/test_model.py test/test_log_prob.py: 33 passed, 2 failed. The failures are test_compile_force and test_model_paths. They fail the same way without this change (my sandbox cannot write to / and has a read-only compile path), so they are unrelated.
  • pytest test/test_model.py -k diagnose without the model.py change: the 3 new tests fail (test_diagnose_error_argument, test_diagnose_removes_output_dir[False], [True]); the existing test_diagnose passes.
  • Same command with the change: 4 passed.
  • isort, flake8, pylint and mypy are clean.

AI disclosure, per the Stan AI Contribution Policy: this change was prepared with Claude Code (Anthropic).

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):

Aleksandr Tarutin

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

🤖 Generated with Claude Code

https://claude.ai/code/session_013imjkuZ1miKJ7HzHWg6Ku2

The error argument was passed to CmdStan as epsilon=<error>. The output
directory was created with mkdtemp and never removed; it is now a
TemporaryDirectory.

Closes stan-dev#871

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013imjkuZ1miKJ7HzHWg6Ku2

This branch has not been deployed

No deployments
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.

diagnose: the error argument is sent as epsilon; the output directory is never removed

1 participant