Use variadic template for basket extensions #85
Conversation
nmellado
force-pushed
the
variadic-basket
branch
2 times, most recently
from
d4ea82d to
7284bd2
Compare
nmellado
force-pushed
the
variadic-basket
branch
from
7284bd2 to
6864cb9
Compare
nmellado
force-pushed
the
variadic-basket
branch
from
6864cb9 to
f23ae09
Compare
|
@ThibaultLejemble could you check if this works with CUDA ? |
I compiled and ran the Cuda example with this new basket and it works fine! |
azaleostu
force-pushed
the
variadic-basket
branch
from
f6afc81 to
c2f77a4
Compare
| template <class, class, typename T> class Forward: public T {}; | ||
| template <class P, class W, | ||
| template <class, class, typename> class... Exts> | ||
| struct BasketAggregate : BasketAggregateImpl<P, W, PrimitiveBase<P, W>, Exts...> |
There was a problem hiding this comment.
What is the purpose of this intermediate struct since the class Basket could just inherits directly from internal::BasketAggregateImpl<P, W, PrimitiveBase<P, W>, Exts...>::type?
There was a problem hiding this comment.
I chose to hide the PrimitiveBase<P, W> part from the Basket class definition as I thought it could be considered an implementation detail, but it is true that what you mentioned is equivalent (I also thought it made the Basket class a bit more readable)
Ponca/src/Fitting/basket.h
Outdated
| template <class P, class W, int Type, | ||
| template <class, class, typename> class Ext0, | ||
| template <class, class, typename> class... Exts> | ||
| class BasketAutoDiff : public internal::BasketAutoDiffAggregate<P, W, Type, Ext0, Exts...>::type |
There was a problem hiding this comment.
Is the AutoDiff feature in draft or can we review it?
Maybe we can move the AutoDiff feature in another PR?
There was a problem hiding this comment.
This was added as a followup to #106. The aggregate part should be ready for review, but I don't know what the functionality of the class is going to be yet, so I can remove this from this PR for now, if necessary.
There was a problem hiding this comment.
@azaleostu could you remove the autodiff version so we can merge this refactoring, and then add autodiff in another PR (with appropriate testing)
azaleostu
force-pushed
the
variadic-basket
branch
from
c2f77a4 to
1dde3ba
Compare
azaleostu
force-pushed
the
variadic-basket
branch
from
1dde3ba to
7933f4c
Compare
CHANGELOG
Outdated
| @@ -31,6 +31,7 @@ improved doc, as well as several bug fixes and API improvements. | |||
| - [fitting] Fix a bug in dtau computation for scale-only derivation (#98) | |||
| - [fitting] Fix a bug in MlsSphereFitDer weight derivatives (#97) | |||
| - [ci] Fix Compiler is out of heap space error on Github (MSVC), by splitting tests (#87) | |||
| - [fitting] Use variadic template for basket extensions (#85) | |||
There was a problem hiding this comment.
Please move this to line 10
* Use variadic template for basket extensions * Update Changelog --------- Co-authored-by: Amael Marquez <[email protected]> Co-authored-by: mazestic <[email protected]>
This PR changes the hard-coded extension list for the
BasketandBasketDifftypes to a variadic list.