Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CmdStan 2.36 no longer requires fixed_param hacks #771

Merged
merged 7 commits into from
Dec 3, 2024
Merged

Conversation

WardBrian
Copy link
Member

Submission Checklist

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

Summary

CmdStan 2.36 allows the HMC samplers with 0 parameters

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

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

@mitzimorris
Copy link
Member

fails on my macbook:

FAILED test/test_sample.py::test_sample_no_params - TypeError: ufunc 'isnan' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''...

using CmdStan 2.36 rc1, I ran the no-param model datagen - and tried to read in the output.csv file with from_csv_file routine:

(cmdstanpy) ~/github/stan-dev/cmdstanpy/test/data (fixes/2.36)> python
Python 3.12.2 | packaged by conda-forge | (main, Feb 16 2024, 20:54:21) [Clang 16.0.6 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import cmdstanpy
>>> foo = cmdstanpy.from_csv('output.csv')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/mitzi/github/stan-dev/cmdstanpy/cmdstanpy/stanfit/__init__.py", line 132, in from_csv
    check_sampler_csv(
  File "/Users/mitzi/github/stan-dev/cmdstanpy/cmdstanpy/utils/stancsv.py", line 24, in check_sampler_csv
    meta = scan_sampler_csv(path, is_fixed_param)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mitzi/github/stan-dev/cmdstanpy/cmdstanpy/utils/stancsv.py", line 80, in scan_sampler_csv
    lineno = scan_hmc_params(fd, dict, lineno)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/mitzi/github/stan-dev/cmdstanpy/cmdstanpy/utils/stancsv.py", line 277, in scan_hmc_params
    metric = config_dict['metric']
             ~~~~~~~~~~~^^^^^^^^^^
KeyError: 'metric'
>>> 

more logic needed in the CSV reader. (sigh)

@WardBrian
Copy link
Member Author

FAILED test/test_sample.py::test_sample_no_params - TypeError: ufunc 'isnan' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''...

weird, that’s the error this branch gets if you run it with an older cmdstan. I don’t get that with the RC

I didn’t try from_csv yet, but I’m a bit surprised at that error, since the changes here still do create a (empty) matrix

@WardBrian
Copy link
Member Author

By far my much bigger concern is stansummary apparently aborting on MacOS — we can fix cmdstanpy stuff separately from the release if needed, but that will hold the release up

@mitzimorris
Copy link
Member

mitzimorris commented Nov 27, 2024

the underlying problem is that std::mcmc::chainset.quantiles function omits to check whether the set of draws are finite and have variance.

this becomes a problem because when there are no parameters, the stepsize__ column is all nans, and stan::math::quantiles aborts (as it should?)

@WardBrian
Copy link
Member Author

After the latest round of Stan PRs, this branch is now passing:

https://github.com/stan-dev/cmdstanpy/actions/runs/12091362509

there might be more fixes needed on our end for things like the from_csv method, but I am guessing that RC2 will be more or less done

@WardBrian WardBrian marked this pull request as ready for review December 3, 2024 15:47
@WardBrian
Copy link
Member Author

@WardBrian WardBrian requested a review from mitzimorris December 3, 2024 15:47
Copy link
Member

@mitzimorris mitzimorris left a comment

Choose a reason for hiding this comment

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

LGTM! nice to be rid of that hack.

cmdstanpy/model.py Show resolved Hide resolved
test/test_sample.py Show resolved Hide resolved
@WardBrian WardBrian merged commit 0ed8811 into develop Dec 3, 2024
16 checks passed
@WardBrian WardBrian deleted the fixes/2.36 branch December 3, 2024 17:32
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.

2 participants