Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Feature/issue 962 bivar norm #1009
Feature/issue 962 bivar norm #1009
Changes from 37 commits
f1964a3
8976a72
26c5a9c
21f2333
c6f9d34
5b7f6f8
d85f493
b427d4f
611636b
e459c7c
ad8dbbc
f078373
d530927
a14dcd4
e01221e
53f31ff
36e85c9
257b1fe
2b8a660
2a36726
abf12bc
82aaaff
315b830
d4c9ad4
1a88b14
b358949
ff53f15
4199f64
a2047c3
464a9b7
786e75f
0ea95ec
a6dd957
132dba6
73a2c3e
1963d81
5c99911
e8ad567
c5cb5a2
36f16f1
0c0ea11
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stan's
is_finite
will work directly ony1
, I think---I'm not sure why there's a recursive value_of here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that Stan has an is_finite implementation that works for arbitrary types. Here's an implementation for
vars
: https://github.com/stan-dev/math/blob/502487c511594ccb93eb979df5b8fe163becb417/stan/math/rev/scal/fun/boost_isfinite.hpp but as far as I can tell this is it. I have thevalue_of_rec
to handle the nested AD types cases.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking. That's a separate shortcoming of the math lib you don't need to address here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arithmetic is left associative, so all the constants should be pushed together, here
(0.5 / ...)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
-0.5
should go on the outside to save a multiplication,There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
square
better to use than direct multiplication? i.e.x * x
vs.square(x)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only if the variable can be an autodiff variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same naming problem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What naming problem are you referring to? I don't see any local variables ending with
_
.