Fix handling of include-paths in compiler options.

[tillahoffmann]

Submission Checklist

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

Summary

Including include-paths in the stanc_options passed to compile currently breaks compilation if include-paths is a list of paths rather than a string with comma-separated paths. This PR adds conditional handling based on the type.

Follow-up question: Calling compile extends self._compiler_options such that the call changes the state of CmdStanModel in addition to compiling the model. Is that intended? My gut feeling would've been that passing in stanc_options would only affect this one call but not future ones.

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

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

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)

[WardBrian]

Follow-up question: Calling compile extends self._compiler_options such that the call changes the state of CmdStanModel in addition to compiling the model. Is that intended? My gut feeling would've been that passing in stanc_options would only affect this one call but not future ones.

This is interesting, I can see it going either way (after all, if you pass these arguments to the constructor they do affect more than just the first compilation).

I have recently been wondering what is the use-case for having an uncompiled CmdStanMCMC object. It's essentially a broken object at that point, since methods like sample() are unavailable.

I think it's worth considering whether we really want to support the ability to call compile on the object after the fact, or if we should move toward an API where the object is constructed either from an executable or a stan file which is immediately compiled. Besides in our tests, I have a hard time seeing use cases for a standalone compile method on the object

[tillahoffmann]

I have recently been wondering what is the use-case for having an uncompiled CmdStanMCMC object. It's essentially a broken object at that point, since methods like sample() are unavailable.

That makes sense; I always forget about the compile = "force" option in the constructor (which is the only reason I use compile(force=True).

[WardBrian]

Yeah, if we did make the change I mentioned then there wouldn't be a compile=False option in the constructor. It might still be useful to have a force_compile argument there which would basically do the same as compile='force'

[tillahoffmann]

Hm, I can't quite figure out why this is failing in CI on macOS (my local environment is also macOS with python 3.10). Any ideas?

[WardBrian]

I think it's because CmdStan 2.33 just released. If you merge in develop it should pass I believe

[codecov-commenter]

Codecov Report

Merging #689 (cf66f6a) into develop (a28bb95) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #689      +/-   ##
===========================================
- Coverage    80.02%   80.02%   -0.01%     
===========================================
  Files           72       72              
  Lines        10953    10962       +9     
===========================================
+ Hits          8765     8772       +7     
- Misses        2188     2190       +2     

see 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[WardBrian]

Thanks!