Skip to content
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 2682 operands and partials 8 #2833

Conversation

Franzi2114
Copy link
Collaborator

Summary

Here, we adapt the operands_and_partials routine such that it works for 8 parameters. We need this feature for issue #2682. This is a split from PR #2822.

We adapted the following files:
-stan/math/prim/functor/operands_and_partials.hpp
-stan/math/rev/functor/operands_and_partials.hpp
-stan/math/fwd/functor/operands_and_partials.hpp
-stan/math/opencl/rev/operands_and_partials.hpp
-test/unit/math/prim/functor/operands_and_partials_test.cpp
-test/unit/math/rev/functor/operands_and_partials_test.cpp

Tests

We also adapted the test files:
-test/unit/math/prim/functor/operands_and_partials_test.cpp
-test/unit/math/rev/functor/operands_and_partials_test.cpp

Side Effects

No.

Release notes

operands_and_partials expanded to accept 8 parameters.

Checklist

@SteveBronder
Copy link
Collaborator

Hi @Franzi2114 I think on my home computer I have a version of operands_and_partials that actually allows for unlimited numbers of arguments through variadic templates. A while back it didn't work because mingw would fail for the tuple, but now that we have moved off of the mingw based on gcc 4.9.3 I think it will work. Do you mind holding off on this till Thursday when I can put a PR with the variadic changes to operands and partials?

@Franzi2114
Copy link
Collaborator Author

Do you mind holding off on this till Thursday

Hey @SteveBronder, extending operands_and_partials to infinitely many inputs sounds much better than just expanding it to 8. Since I need this for my PR #2822, it would be nice to have it as soon as possible. But Thursday is totally fine for me. Your implementation should work with the call in our new wiener_full_lpdf-function, shouldn't it?
Then, your PR will be merged into develop, I merge develop into our feature-branch and then I can use your operands_and_partials, is this correct?

So, shall I close this PR then? Or does somebody else do it? Should we refer to your PR in a few days?

@SteveBronder
Copy link
Collaborator

Hey @SteveBronder, extending operands_and_partials to infinitely many inputs sounds much better than just expanding it to 8. Since I need this for my PR #2822, it would be nice to have it as soon as possible. But Thursday is totally fine for me. Your implementation should work with the call in our new wiener_full_lpdf-function, shouldn't it?

Yes

Then, your PR will be merged into develop, I merge develop into our feature-branch and then I can use your operands_and_partials, is this correct?

Yep!

So, shall I close this PR then? Or does somebody else do it? Should we refer to your PR in a few days?

No if I don't have time to do update the variadic PR or if something bonks in it then we can go with this version.

@SteveBronder
Copy link
Collaborator

@Franzi2114 see the PR below, apologies for the delay. It's a bit of a big PR so it might take till after this coming release for someone to review and merge

#2841

@Franzi2114
Copy link
Collaborator Author

Hi @SteveBronder, what do you think: Will your PR #2841 be finished in the near future so that I can use the functionality for the 7-parameter DDM (PR #2822) or could we prioritize this PR #2833 such that the PR #2822 can move forward?
Here, we made only small changes to the existing operands_and_partials routine. Basically copying existing lines and adapting them for the 6th, 7th, and 8th parameter.

Copy link
Collaborator

@andrjohns andrjohns left a comment

Choose a reason for hiding this comment

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

Thanks @Franzi2114! I'll approve and merge this so it's compatible with your other changes

@andrjohns andrjohns merged commit 3cbf8e1 into stan-dev:develop Jan 2, 2023
@Franzi2114 Franzi2114 deleted the feature/issue-2682-operands-and-partials-8 branch January 2, 2023 10:03
@Franzi2114
Copy link
Collaborator Author

This refers to issue #2698. I just closed that issue.

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.

3 participants