Add missing scalar prim signatures #2612

rok-cesnovar · 2021-11-06T20:36:29Z

Summary

Adds missing scalar implementations for max, min, fmod, atan2 and pow to fix #2611.

Additionally, this adds a test that generates unit tests on the fly. These tests check that all supported stanc3 scalar signatures (excluding those that return complex numbers and _rng functions) compile in the stan::math namespace. Additionally, it also checks that nothing was placed on any var stack (no inputs get promoted to vars).

Example of a test that is generated:

TEST(PrimSigsTests, pow4786) {
  /*
  pow(real, int) => real
   */
  double real0 = 0.4;
  int int1 = 1;
  auto result2 = stan::math::eval(stan::math::pow(real0, int1));
  auto result3 = stan::math::eval(stan::math::pow(real0, int1));
  EXPECT_STAN_EQ(result2, result3);
  EXPECT_EQ(stan::math::ChainableStack::instance_->var_stack_.size(), 0);
  EXPECT_EQ(stan::math::ChainableStack::instance_->var_nochain_stack_.size(), 0);
  EXPECT_EQ(stan::math::ChainableStack::instance_->var_alloc_stack_.size(), 0);
}

The functions are called twice because I wanted to make sure I included the result in an EXPECT test so it does not get optimized out. This might not be needed, but I just wanted to be safe.

The entire test file that is generated is here: prim_sig_test.txt

This test would be run on Github Actions.

Tests

Adds a new signature test that can be run via ./runTests.py test/scalar_prim_sig. This generates a unit test file that checks that all primitive scalar functions compile and that none of them gets promoted to vars.

Side Effects

/

Release notes

Added implementations of max, min, fmod, atan2 and pow for scalar inputs to the stan::math namespace.

Checklist

Math issue Add stub functions for certain std:: functions #2611
Copyright holder: Rok Češnovar

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:
- Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
- Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

…4.1 (tags/RELEASE_600/final)

bob-carpenter · 2021-11-09T19:27:33Z

I can review this.

The current failure is a bit worrisome---it looks like it just failed outright in the LLVM judging from the plea to submit a bug report:

clang: error: unable to execute command: Killed
clang: error: clang frontend command failed due to signal (use -v to see invocation)
clang version 6.0.0-1ubuntu2~16.04.1 (tags/RELEASE_600/final)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
clang: note: diagnostic msg: PLEASE submit a bug report to http://llvm.org/bugs/ and include the crash backtrace, preprocessed source, and associated run script.
clang: note: diagnostic msg: 
********************

PLEASE ATTACH THE FOLLOWING FILES TO THE BUG REPORT:
Preprocessed source(s) and associated run script(s) are located at:
clang: note: diagnostic msg: /tmp/exp_mod_normal_lpdf_test-2506fe.cpp
clang: note: diagnostic msg: /tmp/exp_mod_normal_lpdf_test-2506fe.sh
clang: note: diagnostic msg: 

********************

rok-cesnovar · 2021-11-09T19:29:49Z

Yeah, the tests were already passing but failed upstream because stan-dev/stan#3080 was required. Restarted the tests, should be ready for review.

bob-carpenter

A bunch of picky doc comments and some more substantive testing questions. My main concern is that all the promotion of integers is tested as that's been an ongoing problem for us with the standard library functions. Not something complicated, just integer tests for all arguments. So if it's binary, that's four cases of {double, int} x {double, int}.

stan/math/prim/fun/abs.hpp

stan/math/prim/fun/atan2.hpp

stan/math/prim/fun/fmod.hpp

test/prob/frechet/frechet_ccdf_log_test.hpp

test/prob/pareto_type_2/pareto_type_2_ccdf_log_test.hpp

test/prob/pareto_type_2/pareto_type_2_cdf_test.hpp

test/signature_parser.py

test/unit/math/prim/fun/pow_test.cpp

hsbadr · 2021-11-10T13:46:49Z

@rok-cesnovar JFYI. This breaks the R packages that depend on rstan with, for instance, pow() ambiguity. I'm using the experimental branch of rstan with the nightly version of stanc3.

rok-cesnovar · 2021-11-10T13:50:32Z

Yeah, I know (it also fails upstream for the exact same reason - the fact that some std functions live in the global namespace).

This branch will be merged together accompanied by a change in stanc3 that will add explicit stan::math:: namespace to all generated Stan Math.
See stan-dev/stanc3#1011 and stan-dev/stanc3#997

Also thanks for checking it. I love that we have up-to-date rstan now!

WardBrian · 2021-11-10T20:01:56Z

For the benefit of anyone who is interested primarily in the Math development without following stanc3 super closely, this PR is motivated by an issue that can lead to programs that stanc3 compiles but the c++ compiler can't. For example,

parameters {
  real alpha;
  real beta;
}
transformed parameters {
  real beta_function = beta(alpha, beta)
}

We allow users to define a variable called beta, but without the explicit namespacing this PR allows the C++ compilers fail to compile the function call of beta.

This reverts commit 872803c.

This reverts commit 103cc03. # Conflicts: # runTests.py

…4.1 (tags/RELEASE_600/final)

rok-cesnovar · 2021-11-27T19:06:58Z

This is now ready for re-review. Ignore the test failure as it fails upstream (we will need some coordination to merge this and stan-dev/stanc3#1011 together).

rok-cesnovar · 2021-12-01T09:55:16Z

Please ignore the review request.

Testing with the Stanc3 changes revealed that we also have to add Stan Math support for standard library functions that support std::complex inputs (arg, abs, norm, conj, proj, polar, exp, log, log10, pow, sqrt and trigonometric functions).

The implementations should be simple, just calling the std:: function.

…4.1 (tags/RELEASE_600/final)

rok-cesnovar · 2021-12-23T18:10:08Z

Added stan::math namespaced functions for std::complex<T> inputs, where T is not var for the following: arg, conj, norm, pow, proj.

Also added tests for complex inputs for other complex functions: abs, exp, log, log10, sqrt and trigonometric.

All models now compile with stan-dev/stanc3#1011 and this branch so this is ready for review.

WardBrian · 2022-01-05T14:07:27Z

@bob-carpenter - could you take another look at this?

bob-carpenter

looks good. thanks for dealing with this huge pr.

bob-carpenter · 2022-01-05T16:33:09Z

This should be OK to merge when tests pass.

stan-buildbot · 2022-01-06T14:14:57Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.59	3.57	1.01	0.6% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.93	-7.02% slower
eight_schools/eight_schools.stan	0.09	0.09	0.99	-0.83% slower
gp_regr/gp_regr.stan	0.14	0.14	1.02	1.52% faster
irt_2pl/irt_2pl.stan	5.79	5.85	0.99	-0.99% slower
performance.compilation	93.66	91.01	1.03	2.83% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.04	8.07	1.0	-0.32% slower
pkpd/one_comp_mm_elim_abs.stan	32.1	31.68	1.01	1.32% faster
sir/sir.stan	119.52	121.5	0.98	-1.66% slower
gp_regr/gen_gp_data.stan	0.04	0.03	1.01	1.08% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.02	3.01	1.0	0.25% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.42	0.94	-5.87% slower
arK/arK.stan	2.05	2.06	0.99	-0.62% slower
arma/arma.stan	0.23	0.23	1.0	-0.2% slower
garch/garch.stan	0.57	0.57	1.0	0.23% faster
Mean result: 0.994183784227

Jenkins Console Log
Blue Ocean
Commit hash: 943ccd3

Machine information

ProductName: Mac OS X ProductVersion: 10.11.6 BuildVersion: 15G22010

CPU:
Intel(R) Xeon(R) CPU E5-1680 v2 @ 3.00GHz

G++:
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

Clang:
Apple LLVM version 7.0.2 (clang-700.1.81)
Target: x86_64-apple-darwin15.6.0
Thread model: posix

rok-cesnovar and others added 14 commits November 6, 2021 21:14

write script to generate tests

103cc03

fix 2 typos

a4ddd66

add prim scalar signatures for missing functions

3411c67

fix runtest

e6d68e8

add requires

4075c09

formatting

af70d36

add to GHA

872803c

remove using std::fmod in tests

969d378

fix test namespaces

64fa890

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

02bee11

…4.1 (tags/RELEASE_600/final)

add separate implementation for pow/fmod

d7af743

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

dd142c3

…4.1 (tags/RELEASE_600/final)

fix require for pow

228a05f

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

43c3a99

…4.1 (tags/RELEASE_600/final)

rok-cesnovar mentioned this pull request Nov 9, 2021

add std:: in tests to avoid ambiguity stan-dev/stan#3080

Merged

bob-carpenter requested changes Nov 9, 2021

View reviewed changes

rok-cesnovar and others added 9 commits November 24, 2021 09:05

Revert "add to GHA"

5769a7b

This reverts commit 872803c.

Revert "write script to generate tests"

50e54f7

This reverts commit 103cc03. # Conflicts: # runTests.py

fix doxygen and return types

e60adb1

optional test fixes

b3f9ff3

add integer tests to pow_test

dc23f52

cleanup

8f730bb

Merge commit 'ddf8946198ae531954456996bc2f2cf62374c3e0' into HEAD

2301ba1

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

0f4383d

…4.1 (tags/RELEASE_600/final)

fix pow prim test

465a551

rok-cesnovar and others added 4 commits November 26, 2021 11:17

add sig_test

4a10aee

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

00397e9

…4.1 (tags/RELEASE_600/final)

move test file

79d0351

fix test issue

d9795d2

rok-cesnovar requested a review from bob-carpenter November 27, 2021 19:06

rok-cesnovar and others added 7 commits December 23, 2021 10:49

Merge branch 'develop' into missing_prim_scalar_sigs

ed76fdd

add test for complex args for existing tests

e13933d

add missing implementations for arg, conj, norm, proj

0925494

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

382be99

…4.1 (tags/RELEASE_600/final)

fix missing header in log

aac0a55

fix pow

199da11

[Jenkins] auto-formatting by clang-format version 6.0.0-1ubuntu2~16.0…

27b3748

…4.1 (tags/RELEASE_600/final)

bob-carpenter approved these changes Jan 5, 2022

View reviewed changes

Merge branch 'develop' into missing_prim_scalar_sigs

943ccd3

rok-cesnovar merged commit 4940a43 into develop Jan 6, 2022

rok-cesnovar deleted the missing_prim_scalar_sigs branch January 6, 2022 14:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing scalar prim signatures #2612

Add missing scalar prim signatures #2612

rok-cesnovar commented Nov 6, 2021 •

edited

Loading

bob-carpenter commented Nov 9, 2021

rok-cesnovar commented Nov 9, 2021

bob-carpenter left a comment

hsbadr commented Nov 10, 2021

rok-cesnovar commented Nov 10, 2021

WardBrian commented Nov 10, 2021

rok-cesnovar commented Nov 27, 2021

rok-cesnovar commented Dec 1, 2021

rok-cesnovar commented Dec 23, 2021

WardBrian commented Jan 5, 2022

bob-carpenter left a comment

bob-carpenter commented Jan 5, 2022

stan-buildbot commented Jan 6, 2022

Add missing scalar prim signatures #2612

Add missing scalar prim signatures #2612

Conversation

rok-cesnovar commented Nov 6, 2021 • edited Loading

Summary

Tests

Side Effects

Release notes

Checklist

bob-carpenter commented Nov 9, 2021

rok-cesnovar commented Nov 9, 2021

bob-carpenter left a comment

Choose a reason for hiding this comment

hsbadr commented Nov 10, 2021

rok-cesnovar commented Nov 10, 2021

WardBrian commented Nov 10, 2021

rok-cesnovar commented Nov 27, 2021

rok-cesnovar commented Dec 1, 2021

rok-cesnovar commented Dec 23, 2021

WardBrian commented Jan 5, 2022

bob-carpenter left a comment

Choose a reason for hiding this comment

bob-carpenter commented Jan 5, 2022

stan-buildbot commented Jan 6, 2022

rok-cesnovar commented Nov 6, 2021 •

edited

Loading