Add optional `--data-file` for `--debug-generate-inits`

[nhuurre]

Closes #1317 There's no real error handling here; if the input data does not conform to the expected type it is ignored silently.

Submission Checklist

• [x] Run unit tests
• Documentation
  • [ ] If a user-facing facing change was made, the documentation PR is here:
  • [ ] OR, no user-facing changes were made

Release notes

Fixed an issue with the --debug-generate-inits debugging flag if parameters depended on data for sizes.

Copyright and Licensing

Copyright holder: Niko Huurre

By submitting this pull request, the copyright holder is agreeing to license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

[codecov[bot]]

Codecov Report

Merging #1319 (c08a4af) into master (2081cb6) will decrease coverage by 0.23%. The diff coverage is 53.16%.

@@            Coverage Diff             @@
##           master    #1319      +/-   ##
==========================================
- Coverage   89.01%   88.78%   -0.23%     
==========================================
  Files          64       64              
  Lines        9777     9839      +62     
==========================================
+ Hits         8703     8736      +33     
- Misses       1074     1103      +29     

Impacted Files	Coverage Δ
...analysis_and_optimization/Debug_data_generation.ml	79.46% <44.23%> (-9.99%)	:arrow_down:
src/stanc/stanc.ml	81.42% <69.56%> (+1.26%)	:arrow_up:
src/frontend/Errors.ml	88.46% <75.00%> (-2.45%)	:arrow_down:

[WardBrian]

I agree that I don't think we should be doing input validation, but it would be nice to improve the error message if you don't supply this file. Even with this PR, the result when the argument is not supplied (or, I suppose, if it is, but the required entries are missing) is still

  ("Fatal error: this should never happen. Please file a bug on https://github.com/stan-dev/stanc3/issues/new and include the model that caused this issue."
   "Cannot convert size to number.")

Raised at Base__Error.raise in file "src/error.ml" (inlined), line 8, characters 14-30
Called from Base__Error.raise_s in file "src/error.ml", line 9, characters 19-40
Called from Analysis_and_optimization__Debug_data_generation.unwrap_int_exn in file "src/analysis_and_optimization/Debug_data_generation.ml" (inlined), line 47, characters 38-58
...

Since this is all debugging flags, I don't think we need it to be much better, but something like this in place of the current Fatal_error

      failwith
        (Fmt.str
           "Unable to evaluate expression '%a' down to a number literal. \
            Consider supplying the --debug-data-file argument to supply the \
            necessary variables "
           Expr.Typed.pp e )

Yields the much more informative

Uncaught exception:

  (Failure
   "Unable to evaluate expression ((N + K)) down to a number literal. Consider supplying the --debug-data-file argument to supply the necessary variables ")

Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33
Called from Analysis_and_optimization__Debug_data_generation.unwrap_int_exn in file "src/analysis_and_optimization/Debug_data_generation.ml" (inlined), line 51, characters 38-58

while also not requesting a bug report.

I think we also need to catch any errors in the JS frontend and stringify them.

Also (I may have tipped my hand in the earlier example message), how do you feel about renaming this to --debug-data-file to keep all of these flags with the same prefix?

Finally, we should add some minimal tests. We don't want the output in the test files since it is RNG based, but some simple cram-style tests which pipe the output to dev/null and simply show the return code would be nice, I think. I can write those later if you don't want to now, they're definitely low priority.

[nhuurre]

Good point about fatal errors. I've added some exception-catching. JS frontend should also work now.

[WardBrian]

I put together some really minimal tests: https://github.com/wardbrian/stanc3/tree/tests/debug-data-file/

I decided to use python rather than jq, since I believe our docker image already has it. If they look good to you, mind merging that branch into yours?

[WardBrian]

Huh, maybe we don't have python? (or it's called python3?)