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

Trouble Updating CmdStan to 2.36: C++20 Compatibility Issues #1306

Closed
richardCrandall opened this issue Dec 29, 2024 · 9 comments
Closed

Trouble Updating CmdStan to 2.36: C++20 Compatibility Issues #1306

richardCrandall opened this issue Dec 29, 2024 · 9 comments

Comments

@richardCrandall
Copy link

Summary:

After updating to CmdStan 2.36, I encountered repeated error/warning messages that appear related to C++20 vs. C++17 compatibility. Switching to C++17 did not resolve the issue. Additionally, Stan models now run significantly slower or fail to execute.

Description:

I first experienced this issue with CmdStan 2.35 after updating both CmdStan and CmdStanR to their latest versions. At that time, I resolved it by adding the following line to the make/local file and rebuilding CmdStan:

CXXFLAGS += -std=c++17

This worked for CmdStan 2.35 but no longer resolves the issue in CmdStan 2.36. Rebuilding with this flag still results in the same warnings/errors.

Reproducible Steps:

The issue occurs after uninstalling and reinstalling CmdStan and CmdStanR, followed by rebuilding CmdStan.

Current Output:

During installation/rebuild, I receive numerous warnings. Below are representative snippets:

In file included from stan/lib/stan_math/lib/tbb_2020.3/include/tbb/tbb_profiling.h:123,
from stan/lib/stan_math/lib/tbb_2020.3/include/tbb/task.h:36,
from stan/lib/stan_math/lib/tbb_2020.3/include/tbb/task_arena.h:23,
from stan/lib/stan_math/stan/math/prim/core/init_threadpool_tbb.hpp:18,
from src/cmdstan/arguments/arg_num_threads.hpp:5,
from src/cmdstan/command.hpp:8,
from src/cmdstan/main.cpp:1:
stan/lib/stan_math/lib/tbb_2020.3/include/tbb/atomic.h:422:24: warning: template-id not allowed for constructor in C++20 [-Wtemplate-id-cdtor]
422 | constexpr atomic(const atomic& rhs): internal::atomic_impl(rhs) {}
| ^
stan/lib/stan_math/lib/tbb_2020.3/include/tbb/atomic.h:422:24: note: remove the ‘< >’
stan/lib/stan_math/lib/tbb_2020.3/include/tbb/atomic.h:437:32: warning: template-id not allowed for constructor in C++20 [-Wtemplate-id-cdtor]
437 | constexpr atomic(const atomic& rhs):
| ^
stan/lib/stan_math/lib/tbb_2020.3/include/tbb/atomic.h:454:1: note: in expansion of macro ‘__TBB_DECL_ATOMIC’
454 | __TBB_DECL_ATOMIC(__TBB_LONG_LONG)
| ^~~~~~~~~~~~~~~~~

In file included from stan/lib/stan_math/stan/math/prim/prob/std_normal_ccdf_log.hpp:5,
from stan/lib/stan_math/stan/math/prim/prob.hpp:276,
from stan/lib/stan_math/stan/math/prim.hpp:18,
from stan/src/stan/io/dump.hpp:7,
from src/cmdstan/command_helper.hpp:10,
from src/cmdstan/command.hpp:13:
stan/lib/stan_math/stan/math/prim/prob/std_normal_lccdf.hpp: In function ‘stan::return_type_t stan::math::std_normal_lccdf(const T_y&)’:
stan/lib/stan_math/stan/math/prim/prob/std_normal_lccdf.hpp:52: note: ‘-Wmisleading-indentation’ is disabled from this point onwards, since column-tracking was disabled due to the size of the code/headers
52 | } else if (y_dbl > 8.25) {
stan/lib/stan_math/stan/math/prim/prob/std_normal_lccdf.hpp:52: note: adding ‘-flarge-source-files’ will allow for more column-tracking support, at the expense of compilation time and memory

Expected Output:

Successful installation of CmdStan without errors or warnings.

Current Version:

  • Operating System: Windows 11
  • CmdStan Version: 2.36.0
  • Compiler/Toolkit: GCC 13.3.0; GNU Make 4.4.1
  • RStudio Version: RStudio 2024.12.0
@andrjohns
Copy link
Contributor

Those are only warnings, so your installation should still successfully build.

The C++ standard also has no impact on the speed of running models so it might be a codebase change instead, could you share a model that is running slower now so that we can debug?

@richardCrandall
Copy link
Author

Thanks for the prompt reply!

Here is an example of a model that takes much longer to run now than it did before. (Please note that I'm using the Rethinking package from the McElreath textbook Statistical Rethinking.)

library(rethinking)
N_individuals <- 100
N_scores_per_individual <- 10

ability <- rnorm(N_individuals,0,0.1)
N <- N_scores_per_individual * N_individuals
id <- rep(1:N_individuals,each=N_scores_per_individual)
score <- round( rnorm(N,ability[id],1) , 2 )

d <- data.frame(
  id = id,
  score = score )
m_nopool <- ulam(
  alist(
    score ~ dnorm(mu,sigma),
    mu <- a_id[id],
    a_id[id] ~ dnorm(0,10),
    sigma ~ dcauchy(0,1)
    ),
  data=d , chains=4, cores=4 , log_lik = TRUE)

The model also produces many of the warning messages like the first snippet I quoted above. I can send a full copy of the error message if that would be helpful.

@WardBrian
Copy link
Member

We cannot in general guarantee there are no compiler warnings, especially in our decencies which the snippet here seems to be

@richardCrandall
Copy link
Author

Hi @WardBrian,

Thank you for your response and for clarifying that compiler warnings don’t necessarily prevent successful builds. However, I wanted to highlight that these issues weren’t present in earlier CmdStan versions, such as 2.35, where the workaround of using C++17 resolved compatibility concerns. Given that the same workaround no longer works in 2.36, it feels like a regression, even for free software.

I understand that dependencies can introduce challenges, but the significant slowdown and errors I’m encountering now are unexpected compared to my prior experience. It's making it hard for me to continue in my studies of Stan, as each model now takes so much longer to run that it's no longer practicable for me to study even simple models. I appreciate the team’s hard work and would be happy to assist in debugging to the extent possible (I'm not all that tech saavy, so I don't know how much help I can be! But I'll do my best)

Thanks again for your time and support!

Best regards,
Richard

@WardBrian
Copy link
Member

I’m not sure why the warnings would be different, because the version of the dependency did not change. As for why adding the C++17 flag doesn’t remove it any longer, I’m not sure — we’re defaulting to C++17 in this version anyway.

A slow down is definitely a concern. If you compile the same model under 2.35, it’s automatically faster? Is this for all models or a specific one?

@richardCrandall
Copy link
Author

Hi @WardBrian, thank you for taking the time to follow up on my issue -- I really appreciate your attention to this!

To answer your first question, I actually tried to go back and delete cmdstan 2.36 and replace it with 2.35, but I still got the same slowdown and warnings. Actually I uninstalled R, RStudio, and rtools44 to reinstall everything from the ground up -- as excessive as that may be, it seemed to work when I was first troubleshooting this issue back in November. I really thought that switching back to 2.35 would be at least a short-term fix, so now I'm at a loss for what's going on.

Here's a quick recap of all I can say with at least some certainty:

  • Based on my conversation log with generative AI, these problems began on or around Nov 25, 2024 when I tried to update to the latest version of cmdstan and cmdstanr.
  • At that time, adding the C++17 flag seemed to be the thing that worked.

To your question about all models vs. a specific one, I went back to my notes from Statistical Rethinking and tested the simplest Stan model I could find (shown below). This model also experienced significant slowdown and the same (or at least highly similar) warnings. So it does seem to me that my slowdown issues are fairly general and not limited to one or two complex models.

library(rethinking)
data(rugged)
d <- rugged
d$log_gdp <- log(d$rgdppc_2000)
dd <- d[complete.cases(d$rgdppc_2000), ]
dd$log_gdp_std <- dd$log_gdp/mean(dd$log_gdp)
dd$rugged_std <- dd$rugged/max(dd$rugged)
dd$cid <- ifelse(dd$cont_africa == 1, 1, 2)
dat_slim <- list(log_gdp_std = dd$log_gdp_std, rugged_std = dd$rugged_std,
                 cid = as.integer(dd$cid))
str(dat_slim)
m9.1 <- ulam(
  alist(log_gdp_std ~ dnorm(mu, sigma), mu <- a[cid] + b[cid] * (rugged_std - 0.215),
        a[cid] ~ dnorm(1, 0.1), 
        b[cid] ~ dnorm(0, 0.3), 
        sigma ~ dexp(1)), 
  data = dat_slim, chains = 14,cores = 14)

If there’s anything unclear about my explanation or additional details you’d like me to provide, please let me know -- I’d be happy to elaborate further! Thanks again for your help.

@andrjohns
Copy link
Contributor

@richardCrandall are you using cmdstanr or rstan to compile and run the ulam models? Looking at the documentation, it uses rstan by default.

I've also run your example ulam model with cmdstanr and compared the timings for v2.34.1 and v2.36.0, and they were almost identical for me. Could you give a bit more information about the time that you're expecting the model to take to run? How long did it take before and how long does it take now.

And to emphasise again, you can safely ignore the compiler warnings, they are completely unrelated to model performance and running time

@richardCrandall
Copy link
Author

Hi @andrjohns thanks for your continued help!

I feel a bit like the person who calls the mechanic to come inspect their washing machine only for the thing to run perfectly once the mechanic is in your house. This morning I reran the very same model I had quoted in the above message yesterday -- and now my laptop is able to run it again in a reasonable time and without compiler warnings. (I promise it was giving me trouble yesterday -- I tested it before writing my last post!) My best guess is that a Lenovo driver/system update I ran this morning may have fixed things? I really can't account for why else the model would have malfunctioned yesterday and worked fine today -- I stopped tinkering with R/RStudio/Stan/etc after my post yesterday, so I don't know how else my laptop could have "fixed itself."

Just to address the question of if rethinking uses cmdstanr or rstan, from what I've read rethinking used to use rstan but now it uses cmdnstan: https://github.com/rmcelreath/rethinking/releases/tag/v2.2.1 Also, when I pull up the documentation for rethinking (version 2.42) directly in R, I see it depends on cmdstanr only -- I don't see a mention of rstan anymore. Just sharing this information in the hopes it benefits you, in thanks for all the time you've invested in helping me.

@andrjohns I want to thank you and @WardBrian for the time you have shared with me in trying to solve this issue.

@bob-carpenter
Copy link
Contributor

Thanks for following up, @richardCrandall. I know the feeling of "works with the mechanic watching" from pestering the devs myself.

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

No branches or pull requests

4 participants