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

Doc Fix? Setting STAN_THREADS=false (or FALSE, 0, etc) results in STAN_THREADS=true #1293

Closed
katrinabrock opened this issue Aug 20, 2024 · 3 comments · Fixed by stan-dev/docs#808

Comments

@katrinabrock
Copy link

Summary:

As concisely stated in this comment: #1179 (comment)

Most of our makefile logic just checks if STAN_THREADS is defined, not that it is set to a "truthy" value.

However, the documentation does not explicitly state this, leading users (including apparently cmdstanr developers) to incorrectly infer that falsy values will turn off multi-threading.

Description:

Not sure if the behavior should change or the doc should change here, but certainly if the behavior remains as-is, it would be good to make the doc more explicit. This issue likely impacts other flags as well.

It would also be helpful for make help-dev or a similar command to resolve the value to how it will be interpreted at compile time.

Reproducible Steps:

~/.cmdstan/cmdstan-2.35.0$ STAN_THREADS=false make examples/bernoulli/bernoulli
~/.cmdstan/cmdstan-2.35.0$ examples/bernoulli/bernoulli info|grep STAN_THREADS

Current Output:

STAN_THREADS=true

Expected Output:

STAN_THREADS=false

Current Version:

v2.35.0

@WardBrian
Copy link
Member

I definitely agree this needs to be explained better!

Re:

It would also be helpful for make help-dev or a similar command to resolve the value to how it will be interpreted at compile time.

You can use make print-compiler-flags to see a lot of good info. In particular, if you set STAN_THREADS, the define is included:

...
  - CXXFLAGS_THREADS             -DSTAN_THREADS
...

@katrinabrock
Copy link
Author

ah, thanks! re: make print-compiler-flags

@katrinabrock
Copy link
Author

^ PR for doc fix

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 a pull request may close this issue.

2 participants