-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
iter_warmpup=0
and fixed_param=True
will always fail for stan 2.34.1
#766
Comments
Thanks for reporting. Sounds like a bug we should fix. |
@carrascomj what action would you like to see, given that the latest cmdstan fixed this issue? |
Removing that early return and testing that it satisfies the expectations would make sense. In general, (only in my opinion) silently removing choices of the user is bound to cause troubles; if the API changes in stan again, it will be very difficult to catch this kind of errors that affect only particular combinations of command line options. I pointed out in a different repository (which uses cmdstanpy biosustain/Maud#469 (comment)) that a particular combination of parameters makes As a nice-to-have as an user, it would also be convenient to have a clear statement of Minimum Required Version of Stan supported by the particular version of cmdstanpy. I see that in CI, you test for the latest stan commit, so I guess the policy is to only support the latest Stan (?). |
We test against the latest released version, but we currently support old versions on a best-effort basis, which can be seen by the presence of version-based conditionals in the code. One item under consideration for the upcoming 2.0 release is to define a rolling window where the previous |
Thanks for the explanation. I believe the rolling window is a great idea, although I understand that is quite a huge effort of tinkering with github actions, I'll be looking forward to that! |
Removing the early return seems fine at first glance, since the |
Summary:
If
fixed_param=True
,adapt_engaged
will always be left as default (True) because of this early return https://github.com/stan-dev/cmdstanpy/blob/develop/cmdstanpy/cmdstan_args.py#L361.This is okay for the newer stan since this if-statement. But for previous stan (tried 2.34.1), this will always fail since
adapt_engaged
must be False even whenfixed_param
is supplied.Description:
This only affects previous versions of Stan. I am not sure about the policy of supporting versions but it is a breaking change that will make previously working code fail if cmdstan itself is not updated.
The text was updated successfully, but these errors were encountered: