Possible invalid/incorrect Stan csv used in unit testing

[amas0]

While in the process of working to update some of the Stan CSV I/O functions, I ran into test/data/fixed_param_sample.csv, which seems to only be used in test/test_sample.py::test_from_csv_fixed_param. The naming here seems to imply that this CSV should have come from fixed param sampling?

But within the config block of this CSV file, we have:

#     algorithm = hmc (Default)

So this doesn't seem to actually be a CSV generated via fixed_param sampling? I'm not sure if I'm missing anything here. If so, let me know. If this is a mistake, I can go ahead and update it.

[WardBrian]

I believe that file was representative of one that generated by a model with 0 parameters. Cmdstan would “helpfully” detect this and switch to using the fixed_param sampler, but the CSV header would still report whatever the requested settings were.

The only way you could notice this was that there were fewer diagnostic parameters (only three __ columns, rather than 7, no adaptation comment, and the model would print something to stdout about it as well.

This misfeature was finally removed a few versions ago, but I guess we’ve kept that test file around. We should probably have a more modern equivalent, if not as a replacement at least as an additional test

[amas0]

Ah okay, interesting. Is there a policy on how old cmdstan versions are supported by cmdstanpy? If it's been a number of versions since generating a file like this has been possible, it seems reasonable to drop it in favor of a modern alternative.

I imagine for this test an appropriate csv would be one actually generated with the fixed_param sampler?

[WardBrian]

We do not have an official policy (yet, c.f. #685). You can still find some code that claims to be in support of versions from july 2020:

cmdstanpy/cmdstanpy/stanfit/mcmc.py

Line 537 in 71d97ca

if cmdstan_version_before(2, 24):

I'm in favor of capping this and instituting some kind of "current + 3" scheme in the 2.0 release.

Unfortunately the change for fixed_param no longer being necessary was only one version ago, so we'd probably support it for some time anyway

[amas0]

Ah okay that makes sense -- I'll go ahead and make sure this file structure is properly supported.

Closing since this is intended behavior.