-
-
Notifications
You must be signed in to change notification settings - Fork 183
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
Backporting Eigen code changes #2653
Comments
That is more or less what rstan has done with BH when stuff like this
happens.
…On Wed, Jan 12, 2022 at 11:34 AM Andrew Johnson ***@***.***> wrote:
Description
In order to fix the compile errors with multiplying/solving
CWiseUnaryViews (which originally necessitated the val_op and adj_op
member introductions), I've submitted patches to Eigen's code which are
needed for fixing our implementations.
For reference, the Eigen PR is here:
https://gitlab.com/libeigen/eigen/-/merge_requests/792
And a Math PR that shows tests passing with the code changes is here:
#2604 <#2604>
However, I think we'll need to agree on a method on backporting these
changes until they're in an Eigen release. While the simplest method would
be to modify the Eigen headers included in the Math library directly, this
would break compatibility with rstan, which links against Eigen headers
in an external package.
To work around this, I'm proposing that we include the Eigen headers with
necessary modifications in the Math library itself (I'm proposing creating
a /prim/plugins folder). The existing header guards for these would
prevent the source (unmodified) versions from being included when run via
rstan.
I have a branch with this proposed structure and backported Eigen changes
here:
https://github.com/stan-dev/math/compare/develop...andrjohns:feature/backport-view-stride?expand=1
Thoughts?
Tagging @hsbadr <https://github.com/hsbadr>, @bgoodri
<https://github.com/bgoodri>, and @SteveBronder
<https://github.com/SteveBronder> given their involvement in the rstan
and Eigen future states
Current Version:
v4.2.1
—
Reply to this email directly, view it on GitHub
<#2653>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZ2XKSFGLWIGG4DWJG6OXDUVWUQLANCNFSM5LZNZOAA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Nice to know I'm on the right track! Any chance you could point me towards some of the |
We don't have any of that stuff at the moment because everything in the
latest BH works as far as we know of. But over the years, we sometimes put
modified .hpp files into rstan/rstan/rstan/inst/include/boost_not_in_BH,
which you can see at
https://github.com/stan-dev/rstan/search?q=boost_not_in_BH
|
Perfect, thanks |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
In order to fix the compile errors with multiplying/solving
CWiseUnaryViews
(which originally necessitated theval_op
andadj_op
member introductions), I've submitted patches to Eigen's code which are needed for fixing our implementations.For reference, the Eigen PR is here: https://gitlab.com/libeigen/eigen/-/merge_requests/792
And a Math PR that shows tests passing with the code changes is here: #2604
However, I think we'll need to agree on a method on backporting these changes until they're in an Eigen release. While the simplest method would be to modify the Eigen headers included in the Math library directly, this would break compatibility with
rstan
, which links against Eigen headers in an external package.To work around this, I'm proposing that we include the Eigen headers with necessary modifications in the Math library itself (I'm proposing creating a
/prim/plugins
folder). The existing header guards for these would prevent the source (unmodified) versions from being included when run viarstan
.I have a branch with this proposed structure and backported Eigen changes here: https://github.com/stan-dev/math/compare/develop...andrjohns:feature/backport-view-stride?expand=1
Thoughts?
Tagging @hsbadr, @bgoodri, and @SteveBronder given their involvement in the
rstan
andEigen
future statesCurrent Version:
v4.2.1
The text was updated successfully, but these errors were encountered: