Generalize cauchy by t4c1 · Pull Request #1944 · stan-dev/math

t4c1 · 2020-06-18T09:07:00Z

Summary

Generalizes Cauchy distribution related functions to accept general Eigen expressions.

Tests

Tested with expression testing framework.

Side Effects

None

Release notes

Generalized Cauchy distribution related functions to accept general Eigen expressions.

Checklist

Math issue Generalize matrix function signatures #1470
Copyright holder: Tadej Ciglarič

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

stan-buildbot · 2020-06-19T12:33:43Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.96	4.06	0.98	-2.41% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	1.0	0.32% faster
eight_schools/eight_schools.stan	0.09	0.09	0.99	-1.26% slower
gp_regr/gp_regr.stan	0.19	0.19	0.99	-0.62% slower
irt_2pl/irt_2pl.stan	5.21	5.3	0.98	-1.68% slower
performance.compilation	85.42	84.29	1.01	1.32% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	7.77	7.81	0.99	-0.55% slower
pkpd/one_comp_mm_elim_abs.stan	20.67	21.36	0.97	-3.35% slower
sir/sir.stan	99.99	102.64	0.97	-2.66% slower
gp_regr/gen_gp_data.stan	0.04	0.05	0.98	-1.75% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.03	3.02	1.0	0.35% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.31	0.31	0.99	-1.23% slower
arK/arK.stan	2.44	1.78	1.37	27.14% faster
arma/arma.stan	0.6	0.69	0.86	-15.7% slower
garch/garch.stan	0.59	0.59	1.0	-0.18% slower
Mean result: 1.00688747124

Jenkins Console Log
Blue Ocean
Commit hash: ad59a61

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

stan-buildbot · 2020-06-19T17:18:46Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	4.02	4.05	0.99	-0.68% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.95	-4.74% slower
eight_schools/eight_schools.stan	0.09	0.09	0.98	-2.28% slower
gp_regr/gp_regr.stan	0.19	0.19	0.98	-1.61% slower
irt_2pl/irt_2pl.stan	5.26	5.28	1.0	-0.41% slower
performance.compilation	86.7	84.72	1.02	2.28% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	7.69	7.89	0.97	-2.61% slower
pkpd/one_comp_mm_elim_abs.stan	20.55	20.9	0.98	-1.69% slower
sir/sir.stan	91.34	105.03	0.87	-15.0% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.01	1.11% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.04	3.02	1.01	0.62% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.32	0.31	1.02	1.83% faster
arK/arK.stan	2.45	1.78	1.38	27.45% faster
arma/arma.stan	0.6	0.68	0.88	-13.08% slower
garch/garch.stan	0.58	0.59	0.99	-1.19% slower
Mean result: 1.00290623732

Jenkins Console Log
Blue Ocean
Commit hash: ad59a61

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

stan-buildbot · 2020-06-19T20:31:22Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	4.08	4.08	1.0	-0.16% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.95	-5.09% slower
eight_schools/eight_schools.stan	0.09	0.09	0.97	-2.67% slower
gp_regr/gp_regr.stan	0.19	0.19	1.0	0.33% faster
irt_2pl/irt_2pl.stan	5.27	5.36	0.98	-1.71% slower
performance.compilation	86.2	84.58	1.02	1.89% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	7.7	7.68	1.0	0.19% faster
pkpd/one_comp_mm_elim_abs.stan	20.84	20.64	1.01	0.94% faster
sir/sir.stan	99.58	106.06	0.94	-6.51% slower
gp_regr/gen_gp_data.stan	0.04	0.05	0.94	-6.41% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.03	3.03	1.0	0.27% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.32	0.31	1.02	1.87% faster
arK/arK.stan	2.41	1.77	1.36	26.55% faster
arma/arma.stan	0.59	0.66	0.9	-10.56% slower
garch/garch.stan	0.59	0.59	1.0	0.33% faster
Mean result: 1.00738945125

Jenkins Console Log
Blue Ocean
Commit hash: ad59a61

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

SteveBronder · 2020-06-27T00:00:32Z

Before we rewrite all of these do we want to wait for the var_value stuff? It feels like if we don't we'll end up rewriting these distributions multiple times

t4c1 · 2020-06-27T10:01:06Z

Some functions I have rewritten using Eigen expressions instead of scalar_seq_view (for example cauchy_lpdf). I think these willl need no changes for var_value. Once operands_and_partials and value_of support var_value these should just work.

Other functions still use scalar_seq_view (for example cauchy_cdf). Some of these would be slower using Eigen stuff (due to accessing memory multiple times). I think we can eventually do something for Eigen expressions that will work similarly to multi result kernel in kernel generator to fix that. Other function use some other functions, for which we do not have vectorized versions. I think we will eventually have all probability functions use expressions.

Whatever the reason, these will probably need some changes. I am not sure we can support var_value in scalar_seq_view in efficient fashion. However the changes I am doing now to these functions are pretty much trivial. So it should not matter too much they will need another change later.

stan-buildbot · 2020-08-12T13:35:52Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	4.12	4.13	1.0	-0.13% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.96	-4.09% slower
eight_schools/eight_schools.stan	0.09	0.09	0.99	-1.16% slower
gp_regr/gp_regr.stan	0.2	0.2	0.99	-1.07% slower
irt_2pl/irt_2pl.stan	5.34	5.41	0.99	-1.25% slower
performance.compilation	87.28	85.97	1.02	1.5% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	7.63	7.71	0.99	-1.08% slower
pkpd/one_comp_mm_elim_abs.stan	27.05	26.59	1.02	1.7% faster
sir/sir.stan	135.46	141.94	0.95	-4.78% slower
gp_regr/gen_gp_data.stan	0.04	0.05	0.95	-5.53% slower
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.01	3.03	0.99	-0.72% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.4	0.39	1.03	2.53% faster
arK/arK.stan	1.78	1.78	1.0	0.13% faster
arma/arma.stan	0.69	0.6	1.15	12.73% faster
garch/garch.stan	0.57	0.54	1.07	6.6% faster
Mean result: 1.00569469734

Jenkins Console Log
Blue Ocean
Commit hash: 14f5a54

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

bbbales2

Questions.

bbbales2 · 2020-08-17T20:10:10Z

stan/math/prim/prob/cauchy_lpdf.hpp

  using T_partials_return = partials_return_t<T_y, T_loc, T_scale>;
+  using T_partials_array = Eigen::Array<T_partials_return, Eigen::Dynamic, 1>;
  using std::log;
+  using T_y_ref = ref_type_if_t<!is_constant<T_y>::value, T_y>;


What is the logic here? This is the type that will be responsible for .eval() ing the input right?

Right, we are only evaling this if it is not constant. The reason being is that y_ref seems to be used twice. Once in construction of operands_and_partials and once in creation of y_col. However, prim implementation of operands_and_partials does not actually do anything with the arguemnts. So if y is double it is actually only used once and we don't need to eval it.

Since it will get used at least once, it would always be safe to eval it though. So we can get rid of the conditional and it'll be the same.

Yeah it is safe. However I prefere not to for performance reasons. If we avaluate multiple operations in same expression we need to load the data into cache just once. If we evaluate each operation separately we need to load them every time.

I was looking at ref_type_if_t to see what it did and it looks like it returns a plain_type if it doesn't do a reference type.

That will induce a copy anyway: https://mc-stan.org/math/d2/d08/ref__type_8hpp_source.html

No, it will not. If T_y is an expression T_optionally_ref will be a reference to the expression type and so will be the result of ref_type_if_t. No eval.

Oh is the logic for ref_type_if_t is return a reference type if T is an expression or if the condition is true? (as opposed to only checking the condition)?

The logic is to act as Eigen::Ref if the condition is true or just a regular reference (to expression if that is the argument) if the condition is false. Acting as Eigen::Ref means evaluating expensive expressions and not evaluating trivial ones.

@bbbales2 I hope this is clear now. Or is it ... ?

Yeah it's clear. I delayed cause this (and the other lpdf pulls) are major changes to lpdf and I got nervous and wanted at least #1989 to get in.

Until that is in there's at least a couple weaknesses in the testing framework that let bugs through before (though only one of those is with pmfs/pdfs - #1861).

I didn't get around to breaking that pull up Friday.

stan/math/prim/prob/cauchy_lpdf.hpp

# Conflicts: # test/expressions/stan_math_sigs_exceptions.expected

stan-buildbot · 2020-08-18T15:33:45Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	4.13	4.16	0.99	-0.71% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.96	-4.14% slower
eight_schools/eight_schools.stan	0.09	0.09	0.98	-2.15% slower
gp_regr/gp_regr.stan	0.2	0.19	1.01	1.02% faster
irt_2pl/irt_2pl.stan	5.4	5.4	1.0	0.0% slower
performance.compilation	88.61	86.22	1.03	2.7% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	7.65	7.69	0.99	-0.61% slower
pkpd/one_comp_mm_elim_abs.stan	27.17	26.83	1.01	1.23% faster
sir/sir.stan	128.36	130.57	0.98	-1.73% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.0	0.44% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.03	3.03	1.0	-0.25% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.43	0.91	-9.88% slower
arK/arK.stan	1.79	1.81	0.99	-0.86% slower
arma/arma.stan	0.73	0.59	1.24	19.5% faster
garch/garch.stan	0.53	0.53	1.0	-0.14% slower
Mean result: 1.00692937278

Jenkins Console Log
Blue Ocean
Commit hash: 34c44c2

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

bbbales2

Alright I'm unnervous about #1989. Those tests will get in eventually.

t4c1 added 3 commits June 11, 2020 13:31

generalized cauchy

cdb247a

Merge branch 'develop' into generalize_cauchy

837fee6

added missing include

ad59a61

t4c1 added 2 commits August 10, 2020 12:46

Merge branch 'develop' into generalize_cauchy

d166fa9

enabled testing

14f5a54

t4c1 requested a review from andrjohns August 13, 2020 10:22

bbbales2 requested changes Aug 17, 2020

View reviewed changes

Merge branch 'develop' into generalize_cauchy

34c44c2

# Conflicts: # test/expressions/stan_math_sigs_exceptions.expected

bbbales2 approved these changes Aug 27, 2020

View reviewed changes

bbbales2 merged commit 22b0640 into stan-dev:develop Aug 27, 2020

rok-cesnovar mentioned this pull request Oct 13, 2020

Release of Stan Math 3.4 #2128

Closed

t4c1 deleted the generalize_cauchy branch November 30, 2020 09:24

Uh oh!

Conversation

t4c1 commented Jun 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Side Effects

Release notes

Checklist

Uh oh!

stan-buildbot commented Jun 19, 2020

Uh oh!

stan-buildbot commented Jun 19, 2020

Uh oh!

stan-buildbot commented Jun 19, 2020

Uh oh!

SteveBronder commented Jun 27, 2020

Uh oh!

t4c1 commented Jun 27, 2020

Uh oh!

stan-buildbot commented Aug 12, 2020

Uh oh!

bbbales2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stan-buildbot commented Aug 18, 2020

Uh oh!

bbbales2 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

t4c1 commented Jun 18, 2020 •

edited

Loading