Skip to content

Cleanup/1886 cleanup global space in prim tests, replace pull_msg with expect_throw_msg #1947

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

Merged
merged 36 commits into from
Jul 2, 2020

Conversation

rok-cesnovar
Copy link
Member

@rok-cesnovar rok-cesnovar commented Jun 22, 2020

Summary

This PR advances #1886 with the following:

  • cleaned up /prim tests files of global using statements
  • remove the unnecesarry pull_msg functions in the gp_* tests. They can be replaced with EXPECT_THROW_MSG.
  • puts functions with non-unique names file f, fun, f1 in namespaces (The namespace names are based on the file name)
  • removed global variables like const char* function_ = "function"; from the global namespace of tests
  • moved util functions like correct_type_vector in the utils header file
  • added missing header guards to test utility files (ode mock observers and hmm test utilities)

No release notes as this is part of an ongoing project that is being done over multiple PRs. Will put release notes in the final one.

Tests

/

Side Effects

/

Checklist

  • Math issue Organize and cleanup the use of utils in test/ #1886

  • Copyright holder: Rok Češnovar, Univ. of Ljubljana

    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/)

  • the basic tests are passing

    • unit tests pass (to run, use: ./runTests.py test/unit)
    • header checks pass, (make test-headers)
    • dependencies checks pass, (make test-math-dependencies)
    • docs build, (make doxygen)
    • code passes the built in C++ standards checks (make cpplint)
  • the code is written in idiomatic C++ and changes are documented in the doxygen

  • the new changes are tested

rok-cesnovar and others added 30 commits May 26, 2020 16:27
…-part2"

This reverts commit 609c7a6, reversing
changes made to 2e3f863.

# Conflicts:
#	runTests.py
This reverts commit 3f7c339.
@rok-cesnovar rok-cesnovar marked this pull request as ready for review June 23, 2020 12:50
@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.94 4.05 1.22 17.99% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.03 2.94% faster
eight_schools/eight_schools.stan 0.21 0.09 2.31 56.65% faster
gp_regr/gp_regr.stan 0.2 0.2 1.03 3.14% faster
irt_2pl/irt_2pl.stan 5.57 5.5 1.01 1.24% faster
performance.compilation 89.61 87.0 1.03 2.91% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.86 7.93 0.99 -0.83% slower
pkpd/one_comp_mm_elim_abs.stan 27.12 20.99 1.29 22.6% faster
sir/sir.stan 113.21 91.12 1.24 19.51% faster
gp_regr/gen_gp_data.stan 0.05 0.05 1.03 2.5% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.18 3.09 1.03 2.77% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.61 0.33 1.85 46.0% faster
arK/arK.stan 2.86 2.48 1.15 13.13% faster
arma/arma.stan 0.67 0.62 1.08 7.56% faster
garch/garch.stan 1.1 0.6 1.84 45.55% faster
Mean result: 1.27556116968

Jenkins Console Log
Blue Ocean
Commit hash: 6fe1529


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 rok-cesnovar mentioned this pull request Jun 23, 2020
t4c1
t4c1 previously approved these changes Jul 2, 2020
Comment on lines 201 to 202
EXPECT_THROW_MSG(stan::math::gp_exponential_cov(x_2, x_2, sigma_bad, l_bad),
std::domain_error, " magnitude");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[optional] I think we should remove all these tests that expect specific error message when multiple arguments have errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, yeah. Will change just to expect throw.

namespace stan {
namespace test {
template <int R, int C>
void correct_type_vector(const Eigen::Matrix<double, R, C>& x) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[optional] I think a better name would be expect_type_vector.

Copy link
Contributor

@t4c1 t4c1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few more of these tests that expect specific messages from multiple errors

@rok-cesnovar
Copy link
Member Author

Thanks. Done.

@stan-buildbot
Copy link
Contributor


Name Old Result New Result Ratio Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan 4.13 4.04 1.02 2.3% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan 0.02 0.02 1.0 -0.4% slower
eight_schools/eight_schools.stan 0.09 0.09 1.02 2.01% faster
gp_regr/gp_regr.stan 0.19 0.19 0.99 -0.99% slower
irt_2pl/irt_2pl.stan 5.23 5.37 0.97 -2.58% slower
performance.compilation 85.68 84.9 1.01 0.91% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan 7.7 7.68 1.0 0.25% faster
pkpd/one_comp_mm_elim_abs.stan 20.36 20.96 0.97 -2.95% slower
sir/sir.stan 103.29 101.8 1.01 1.44% faster
gp_regr/gen_gp_data.stan 0.04 0.05 0.98 -1.61% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan 3.19 3.02 1.06 5.34% faster
pkpd/sim_one_comp_mm_elim_abs.stan 0.32 0.32 0.99 -0.66% slower
arK/arK.stan 2.42 2.42 1.0 0.0% slower
arma/arma.stan 0.6 0.6 1.0 -0.07% slower
garch/garch.stan 0.52 0.52 1.0 0.28% faster
Mean result: 1.00259275639

Jenkins Console Log
Blue Ocean
Commit hash: 63b873e


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 rok-cesnovar merged commit 7a800b6 into develop Jul 2, 2020
@rok-cesnovar rok-cesnovar deleted the cleanup/1886-test-util-cleanup-part2 branch July 2, 2020 19:09
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 this pull request may close these issues.

4 participants