Closed andrjohns closed 1 month ago
Name | Old Result | New Result | Ratio | Performance change( 1 - new / old ) |
---|---|---|---|---|
arma/arma.stan | 0.19 | 0.18 | 1.04 | 4.05% faster |
low_dim_corr_gauss/low_dim_corr_gauss.stan | 0.01 | 0.01 | 1.02 | 2.27% faster |
gp_regr/gen_gp_data.stan | 0.02 | 0.02 | 1.03 | 2.77% faster |
gp_regr/gp_regr.stan | 0.11 | 0.1 | 1.05 | 4.88% faster |
sir/sir.stan | 76.57 | 75.25 | 1.02 | 1.72% faster |
irt_2pl/irt_2pl.stan | 3.91 | 3.95 | 0.99 | -1.02% slower |
eight_schools/eight_schools.stan | 0.06 | 0.05 | 1.07 | 6.74% faster |
pkpd/sim_one_comp_mm_elim_abs.stan | 0.25 | 0.25 | 1.01 | 1.46% faster |
pkpd/one_comp_mm_elim_abs.stan | 17.99 | 18.3 | 0.98 | -1.7% slower |
garch/garch.stan | 0.45 | 0.46 | 0.98 | -1.54% slower |
low_dim_gauss_mix/low_dim_gauss_mix.stan | 2.77 | 2.82 | 0.98 | -1.71% slower |
arK/arK.stan | 1.72 | 1.67 | 1.03 | 3.04% faster |
gp_pois_regr/gp_pois_regr.stan | 2.53 | 2.58 | 0.98 | -1.93% slower |
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan | 9.07 | 9.35 | 0.97 | -3.1% slower |
performance.compilation | 173.75 | 178.03 | 0.98 | -2.46% slower |
Mean result: 1.0099592951445922
Jenkins Console Log Blue Ocean Commit hash: b62657db8079bb06c39c70bb82d88755522d5b74
Turns out the RTools44 GCC has a bug in its exp(std::complex<double>)
implementation, which is the cause of the mix
test failures.
The result of exp(std::complex<double>(inf))
should be {inf, 0}
as seen on godbolt, but the RTools44 gcc returns {inf, nan}
.
I've notified the toolchain maintainer
Looks like the reason why the RTools40 toolchain was passing but not Rtools44, is that the msys2-based gcc (like rtools40 and those installed via pacman
) define _GLIBCXX11_USE_C99_COMPLEX
by default, but the MXE-based gcc (RTools42-44) don't.
I'm still investigating whether this is an rtools-specific or a general mxe issue, but for now we can just add -D_GLIBCXX11_USE_C99_COMPLEX
to the windows flags
Name | Old Result | New Result | Ratio | Performance change( 1 - new / old ) |
---|---|---|---|---|
arma/arma.stan | 0.19 | 0.18 | 1.05 | 4.87% faster |
low_dim_corr_gauss/low_dim_corr_gauss.stan | 0.01 | 0.01 | 1.06 | 5.28% faster |
gp_regr/gen_gp_data.stan | 0.02 | 0.02 | 1.07 | 6.52% faster |
gp_regr/gp_regr.stan | 0.11 | 0.1 | 1.06 | 5.49% faster |
sir/sir.stan | 79.05 | 75.22 | 1.05 | 4.85% faster |
irt_2pl/irt_2pl.stan | 3.91 | 3.93 | 0.99 | -0.54% slower |
eight_schools/eight_schools.stan | 0.06 | 0.05 | 1.06 | 5.79% faster |
pkpd/sim_one_comp_mm_elim_abs.stan | 0.26 | 0.25 | 1.02 | 2.04% faster |
pkpd/one_comp_mm_elim_abs.stan | 18.31 | 18.21 | 1.01 | 0.53% faster |
garch/garch.stan | 0.47 | 0.46 | 1.01 | 1.36% faster |
low_dim_gauss_mix/low_dim_gauss_mix.stan | 2.85 | 2.85 | 1.0 | 0.01% faster |
arK/arK.stan | 1.64 | 1.67 | 0.98 | -1.78% slower |
gp_pois_regr/gp_pois_regr.stan | 2.55 | 2.54 | 1.0 | 0.48% faster |
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan | 9.16 | 9.28 | 0.99 | -1.28% slower |
performance.compilation | 181.61 | 180.96 | 1.0 | 0.36% faster |
Mean result: 1.0240269407895728
Jenkins Console Log Blue Ocean Commit hash: 39b1ef439f847a0dfe31f525f78a5affade57b4d
Looks like the reason why the RTools40 toolchain was passing but not Rtools44, is that the msys2-based gcc (like rtools40 and those installed via
pacman
) define_GLIBCXX11_USE_C99_COMPLEX
by default, but the MXE-based gcc (RTools42-44) don't.I'm still investigating whether this is an rtools-specific or a general mxe issue, but for now we can just add
-D_GLIBCXX11_USE_C99_COMPLEX
to the windows flags
I've heard back from the RTools maintainer that they would prefer to wait until the next major RTools release before adding this flag, but that I should report the bug upstream to gcc/libstdc++. So we'll need to manually add the flag in the interim.
@WardBrian / @rok-cesnovar Should I keep the makefile as unconditionally adding the flag on Windows (like it is now)? Alternatives that I can think of would be to:
$(echo | g++ -E -dM - | findstr _GLIBCXX11_USE_C99_COMPLEX)
)$(gcc -v | findstr mxe)
)Thoughts/preferences?
I think it's fine being unconditional on Windows
@WardBrian ok to approve and merge before the freeze? Just want to make sure this in now that the next release is recommending the stock RTools toolchain
Summary
Our Github CI for Windows is significantly out of date with the current RTools windows toolchain, this PR updates the workflows to run using the latest RTools44 toolchain
Tests
N/A
Side Effects
N/A
Release notes
Update Github CI to use RTools44 toolchain on windows
Checklist
[x] Copyright holder: Andrew Johnson
The copyright holder is typically you or your assignee, such as a university or company. By submitting this pull request, the copyright holder is agreeing to the license the submitted work under the following licenses:
[x] the basic tests are passing
./runTests.py test/unit
)make test-headers
)make test-math-dependencies
)make doxygen
)make cpplint
)[x] the code is written in idiomatic C++ and changes are documented in the doxygen
[x] the new changes are tested