Feature: L1 and L2 norms #2636

lyndond · 2021-12-29T20:44:06Z

Summary

Adds L1- and L2-norm functionality (described in L1 and L2 norms (Euclidean and taxicab lengths) #2562)
Includes autodiff test framework tests
I would appreciate if someone could look over my rev/.../norm1.hpp and rev/.../norm2.hpp implementations.
- The derivatives are straightforward but I'm still new to the stan autodiff framework so I'm not confident I did them correctly
@bob-carpenter mentioned: using these new functions, we should "replace other uses that calculate vector lengths with norm2()"
- To do this I'll need some guidance on where this calculation might arise in the repo

As noted in the issue:

norm1(x)  =def=  sum(abs(x))
norm2(x)  =def=  sqrt(sum(square(x)))

d/dx norm1(x) = signum(x)
d/dx norm2(x) = x / norm2(x)

Tests

Tests are pretty similar to dot_self
Includes expected behaviour, as well as nan-handling

Side Effects

n/a

Release notes

n/a

Checklist

Math issue L1 and L2 norms (Euclidean and taxicab lengths) #2562
Copyright holder: Lyndon Duong

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

…4.1 (tags/RELEASE_600/final)

andrjohns · 2021-12-30T01:20:12Z

Thanks for the PR @lyndond and welcome to Stan Math! Just a couple of quick changes and then I can do a full review for you. For your implementations, you don't need separate specialisations for std::vector and Eigen types. The Math library has a helper framework apply_vector_unary which allows you to only specify the code for Eigen types, and it will automatically Map std::vector inputs.

Additionally, using the auto return type with Eigen expressions can have unintended side effects, as covered in this help topic and a practical example in #1775. To avoid these, the Math library has an additional framework, Holder, to correctly manage returning Eigen expressions.

You can see an example of applying both of these approaches in the log_softmax header. Can you make those changes and then I'll do a full review?

Thanks!

…4.1 (tags/RELEASE_600/final)

lyndond · 2021-12-30T01:24:53Z

Thanks @andrjohns. I just made a few edits and pushed before reading your comment so I'll make changes according to your suggestions.

…_l2_norms

lyndond · 2021-12-30T02:54:54Z

@andrjohns I followed your tips and reduced some redundancy using apply_vector_unary::reduce.

I noticed that the Jenkins test is failing but can't decrypt the error message. Would you be able to help me with that?

andrjohns · 2021-12-30T02:58:51Z

As far as I can see the Jenkins run hasn't failed, there just isn't a machine available to run the tests at the moment. But it's in the queue and will be run once a machine frees up

…4.1 (tags/RELEASE_600/final)

…tu2~16.04.1 (tags/RELEASE_600/final)" This reverts commit dfc9a8a.

…4.1 (tags/RELEASE_600/final)

stan-buildbot · 2021-12-31T03:51:16Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.6	3.62	1.0	-0.5% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.96	-3.77% slower
eight_schools/eight_schools.stan	0.09	0.08	1.01	1.05% faster
gp_regr/gp_regr.stan	0.15	0.14	1.05	5.17% faster
irt_2pl/irt_2pl.stan	5.71	5.69	1.0	0.4% faster
performance.compilation	93.19	91.19	1.02	2.15% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.09	8.11	1.0	-0.2% slower
pkpd/one_comp_mm_elim_abs.stan	32.44	31.94	1.02	1.54% faster
sir/sir.stan	119.2	123.54	0.96	-3.64% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.03	3.08% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.02	2.99	1.01	0.78% faster
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.4	0.97	-3.4% slower
arK/arK.stan	2.06	2.09	0.98	-1.57% slower
arma/arma.stan	0.23	0.23	1.01	0.73% faster
garch/garch.stan	0.58	0.58	0.99	-0.97% slower
Mean result: 1.00115510181

Jenkins Console Log
Blue Ocean
Commit hash: b745142

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

andrjohns

Thanks for this PR @lyndond, you've done a great job of picking up our frameworks and coding styles!

I've made some requests in this PR. Additionally, you'll also need to add a fwd specialisation for these functions. The fwd specialisation is used for forward-mode autodiff where the values and gradients are computed in the same pass, we use the variable type fvar<T> for these, where T can be a double, var, or another fvar<T>. There's an example of how to specify the fwd specialisation for these kinds of functions in the log_sum_exp header, and feel free to reach out if you need any clarifications or help.

stan/math/rev/fun/norm1.hpp

stan/math/rev/fun/norm2.hpp

stan/math/prim/fun/norm1.hpp

…4.1 (tags/RELEASE_600/final)

lyndond · 2021-12-31T16:53:17Z

Thanks for the review so far @andrjohns.

I made changes according to your suggestions and will work on the corresponding fwd implementations next. Happy New Year in the meantime :)

…4.1 (tags/RELEASE_600/final)

…_l2_norms

bob-carpenter · 2021-12-31T21:38:28Z

@andrjohns: Can't we use the primitive implementation for forward mode? There's not nearly as much efficiency savings in custom autodiff for forward mode because the arithmetic doesn't incur virtual function call overhead (pointer chasing) like in reverse mode. I was thinking there could be a way to use expression templates to select primitive or forward-mode (not reverse mode).

bob-carpenter · 2021-12-31T21:39:20Z

Oh, and @lyndond, if it's passing mix tests, the forward mode's working, as those test everything but the double-based implementations.

lyndond · 2021-12-31T21:51:35Z

@bob-carpenter, great. The rev and templated prim functions were indeed passing all mix tests before I explicitly implemented the fwd functions.

It's unclear to me whether or not the new fwd functions are being tested in the mix tests.

stan-buildbot · 2022-01-01T02:03:17Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.56	3.58	0.99	-0.63% slower
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-1.44% slower
eight_schools/eight_schools.stan	0.09	0.09	1.02	2.16% faster
gp_regr/gp_regr.stan	0.15	0.14	1.03	2.73% faster
irt_2pl/irt_2pl.stan	5.73	5.77	0.99	-0.63% slower
performance.compilation	93.62	90.97	1.03	2.83% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.18	8.14	1.01	0.51% faster
pkpd/one_comp_mm_elim_abs.stan	31.41	31.43	1.0	-0.06% slower
sir/sir.stan	118.77	117.97	1.01	0.68% faster
gp_regr/gen_gp_data.stan	0.03	0.03	1.01	1.03% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	2.99	3.0	1.0	-0.49% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.39	0.39	1.0	-0.45% slower
arK/arK.stan	2.11	2.1	1.01	0.55% faster
arma/arma.stan	0.24	0.23	1.02	2.23% faster
garch/garch.stan	0.59	0.58	1.01	1.15% faster
Mean result: 1.00698312707

Jenkins Console Log
Blue Ocean
Commit hash: 548bccf

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

lyndond · 2022-01-01T21:42:19Z

Turns out my header guards were wrong in my forward implementations so the mix tests were still defaulting to using my templated prim functions instead of the new fwd functions. I've now fixed the header guards but am iffy about the template type trait juggling I had to do in order to resolve function ambiguities for the mix tests to pass.

I'm paranoid the tests are still using the prim implementations so I'd greatly appreciate if someone could confirm my template arguments in the prim and fwd functions make sense!

andrjohns · 2022-01-02T00:30:31Z

@andrjohns: Can't we use the primitive implementation for forward mode? There's not nearly as much efficiency savings in custom autodiff for forward mode because the arithmetic doesn't incur virtual function call overhead (pointer chasing) like in reverse mode. I was thinking there could be a way to use expression templates to select primitive or forward-mode (not reverse mode).

Fair point, I was just erring on the side of consistency between all implementations

andrjohns

One last request and then this is all good to go!

Thanks for sticking through these requests!

andrjohns · 2022-01-02T00:36:55Z

stan/math/prim/fun/norm1.hpp

+ * @return L1 norm of v.
+ */
+template <typename Container, require_container_t<Container>* = nullptr,
+          require_not_st_var<Container>* = nullptr,


These templates can be a little stricter by specifying that they're only for arithmetic types, see the log_softmax header for an example

Cool, I changed the templates to include require_st_arithmetic<> to match the template of log_softmax.

Does require_st_arithmetic<> do the job of both require_not_st_var<> and require_not_st_fvar<>? These are what I used to resolve ambiguity between prim vs rev and prim vs fwd, respectively.

Yep, it will restrict to only matching inputs that satisfy std::is_arithmetic

stan-buildbot · 2022-01-02T09:40:57Z

Name	Old Result	New Result	Ratio	Performance change( 1 - new / old )
gp_pois_regr/gp_pois_regr.stan	3.69	3.57	1.03	3.26% faster
low_dim_corr_gauss/low_dim_corr_gauss.stan	0.02	0.02	0.99	-0.56% slower
eight_schools/eight_schools.stan	0.09	0.09	0.97	-2.58% slower
gp_regr/gp_regr.stan	0.15	0.15	0.96	-4.02% slower
irt_2pl/irt_2pl.stan	5.74	5.75	1.0	-0.22% slower
performance.compilation	93.53	91.4	1.02	2.27% faster
low_dim_gauss_mix_collapse/low_dim_gauss_mix_collapse.stan	8.02	8.11	0.99	-1.12% slower
pkpd/one_comp_mm_elim_abs.stan	31.94	32.81	0.97	-2.73% slower
sir/sir.stan	117.44	119.56	0.98	-1.8% slower
gp_regr/gen_gp_data.stan	0.04	0.04	1.01	0.97% faster
low_dim_gauss_mix/low_dim_gauss_mix.stan	3.06	3.06	1.0	-0.22% slower
pkpd/sim_one_comp_mm_elim_abs.stan	0.38	0.39	0.98	-2.46% slower
arK/arK.stan	2.06	2.06	1.0	-0.11% slower
arma/arma.stan	0.23	0.23	1.0	-0.27% slower
garch/garch.stan	0.58	0.59	0.98	-1.57% slower
Mean result: 0.99295550815

Jenkins Console Log
Blue Ocean
Commit hash: 535f0cb

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

andrjohns

This all looks good to me! Feel free to merge once tests pass.

Thanks for your contribution and congrats on your first Math PR!

rok-cesnovar · 2022-01-02T09:48:08Z

Congrats @lyndond on your first merged PR in Stan Math 🎉. Thanks!

@andrjohns I think someone with elevated permission will have to merge.

andrjohns · 2022-01-02T10:04:00Z

Oh right, will do that now

lyndond · 2022-01-02T14:46:44Z

Great! Thanks @rok-cesnovar @bob-carpenter & @andrjohns. I appreciate the welcoming support and look forward to contributing more in the future.

yizhang-yiz · 2022-04-29T15:47:26Z

@andrjohns where can I find unit tests for rev? I could only find prim & mix but no tests for var type.

rok-cesnovar · 2022-04-29T15:52:13Z

If there are mix tests, then there is no need for separate rev tests - the mix tests both rev, fwd and mix.

If you want to run mix tests but only actually run the rev part of them, set -DSTAN_MATH_TESTS_REV_ONLY to CXXFLAGS.
See https://github.com/stan-dev/math/blob/develop/test/unit/math/test_ad.hpp for details.

yizhang-yiz · 2022-04-29T15:57:55Z

I missed the expect_ad line. Thanks @rok-cesnovar

lyndond and others added 10 commits December 29, 2021 14:13

working L1 norm and tests

47c5bd8

fix typo

7dbd6a9

norm2 and tests done

f89d5f0

revert back dot_self

d27f237

pass lint check

71b3181

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

c6721c6

…4.1 (tags/RELEASE_600/final)

make dot_self templated

e54389c

template and Eigen::Map norm1,norm2

a504f17

clean up tests

94b5ff3

rebase

1b991ad

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

fc9d2ad

…4.1 (tags/RELEASE_600/final)

lyndond added 3 commits December 29, 2021 21:39

reduce redundancy by using apply_vector_unary

fe3be43

Merge branch 'l1_l2_norms' of https://github.com/lyndond/math into l1…

2d234bb

…_l2_norms

cpplint

a7aaa48

lyndond and others added 7 commits December 29, 2021 22:15

change dot_self.hpp back bc no longer used in norms

9b3c93d

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

dfc9a8a

…4.1 (tags/RELEASE_600/final)

Revert "[Jenkins] auto-formatting by clang-format version 6.0.0-1ubun…

8e70232

…tu2~16.04.1 (tags/RELEASE_600/final)" This reverts commit dfc9a8a.

add require_not_st_var to prim funs to fix rev tests

1b3ffc9

Merge commit 'a43562ea29ef1bb892cb7942787d682f002dfc7c' into HEAD

665d254

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

ce03d8b

…4.1 (tags/RELEASE_600/final)

fix broken var in norm2

b745142

andrjohns requested changes Dec 31, 2021

View reviewed changes

stan/math/rev/fun/norm1.hpp Outdated Show resolved Hide resolved

stan/math/rev/fun/norm1.hpp Outdated Show resolved Hide resolved

stan/math/rev/fun/norm2.hpp Show resolved Hide resolved

stan/math/prim/fun/norm1.hpp Outdated Show resolved Hide resolved

lyndond and others added 3 commits December 31, 2021 10:06

remove unnecessary includes

100eb4c

commit suggestions by @andrjohns

19e4736

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

fcbe661

…4.1 (tags/RELEASE_600/final)

lyndond and others added 4 commits December 31, 2021 14:59

added forward-mode specializations

7adbc75

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

18bee81

…4.1 (tags/RELEASE_600/final)

template argument fix

d731ef2

Merge branch 'l1_l2_norms' of https://github.com/lyndond/math into l1…

548bccf

…_l2_norms

forward specializations

61fc4ab

andrjohns requested changes Jan 2, 2022

View reviewed changes

lyndond added 2 commits January 1, 2022 21:59

require_st_arithmetic for prim functions

238420e

replace omitted require_container_t

535f0cb

andrjohns approved these changes Jan 2, 2022

View reviewed changes

andrjohns merged commit 40e2a30 into stan-dev:develop Jan 2, 2022

This was referenced Jan 2, 2022

Add norm1() and norm2() stan-dev/stanc3#1083

Closed

L1 and L2 norms (Euclidean and taxicab lengths) #2562

Open

lyndond mentioned this pull request Jan 9, 2022

Replace sqrt(dot_self()) with norm2 #2648

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: L1 and L2 norms #2636

Feature: L1 and L2 norms #2636

lyndond commented Dec 29, 2021 •

edited

Loading

andrjohns commented Dec 30, 2021

lyndond commented Dec 30, 2021

lyndond commented Dec 30, 2021

andrjohns commented Dec 30, 2021

stan-buildbot commented Dec 31, 2021

andrjohns left a comment

lyndond commented Dec 31, 2021

bob-carpenter commented Dec 31, 2021

bob-carpenter commented Dec 31, 2021

lyndond commented Dec 31, 2021 •

edited

Loading

stan-buildbot commented Jan 1, 2022

lyndond commented Jan 1, 2022

andrjohns commented Jan 2, 2022

andrjohns left a comment

andrjohns Jan 2, 2022

lyndond Jan 2, 2022

andrjohns Jan 2, 2022

stan-buildbot commented Jan 2, 2022

andrjohns left a comment

rok-cesnovar commented Jan 2, 2022

andrjohns commented Jan 2, 2022

lyndond commented Jan 2, 2022

yizhang-yiz commented Apr 29, 2022

rok-cesnovar commented Apr 29, 2022

yizhang-yiz commented Apr 29, 2022

Feature: L1 and L2 norms #2636

Feature: L1 and L2 norms #2636

Conversation

lyndond commented Dec 29, 2021 • edited Loading

Summary

Tests

Side Effects

Release notes

Checklist

andrjohns commented Dec 30, 2021

lyndond commented Dec 30, 2021

lyndond commented Dec 30, 2021

andrjohns commented Dec 30, 2021

stan-buildbot commented Dec 31, 2021

andrjohns left a comment

Choose a reason for hiding this comment

lyndond commented Dec 31, 2021

bob-carpenter commented Dec 31, 2021

bob-carpenter commented Dec 31, 2021

lyndond commented Dec 31, 2021 • edited Loading

stan-buildbot commented Jan 1, 2022

lyndond commented Jan 1, 2022

andrjohns commented Jan 2, 2022

andrjohns left a comment

Choose a reason for hiding this comment

andrjohns Jan 2, 2022

Choose a reason for hiding this comment

lyndond Jan 2, 2022

Choose a reason for hiding this comment

andrjohns Jan 2, 2022

Choose a reason for hiding this comment

stan-buildbot commented Jan 2, 2022

andrjohns left a comment

Choose a reason for hiding this comment

rok-cesnovar commented Jan 2, 2022

andrjohns commented Jan 2, 2022

lyndond commented Jan 2, 2022

yizhang-yiz commented Apr 29, 2022

rok-cesnovar commented Apr 29, 2022

yizhang-yiz commented Apr 29, 2022

lyndond commented Dec 29, 2021 •

edited

Loading

lyndond commented Dec 31, 2021 •

edited

Loading