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

Support custom parameter in sequence template functions #331

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Crydsch
Copy link
Contributor

@Crydsch Crydsch commented Aug 26, 2023

This PR enables custom parameters in the template functions sequence.record(..), sequence.eval(..) and sequence.evalAsync(..) by replacing the specializations with a more generic approach.

In theory the compiler should be able to deduce the correct Constructor of the passed Operation class if there is just one. However it seems to require at least the first argument type to correctly deduce the constructor when given an initializer list like eval<op>({...}).
Currently this done with template specializations that passes either std::vector<std::shared_ptr<Tensor>> tensors or std::shared_ptr<Algorithm> algorithm to disambiguate the templates.

I found a way to supply this first parameter type through the respective Operations class itself, rather than hard-coding them as template specializations.
This makes the code a little compacter, but more importantly it allows custom user operations to use the same template and thus, nice syntax.

This turns

std::shared_ptr<OpCustom> op{ new OpCustom({CustomParameter}) };
sequence->eval(op);

into

sequence->eval<OpCustom>({CustomParameter});

The only change to existing operations is the addition of the first constructor type to each class.
The current functionality is not changed otherwise.

Signed-off-by: crydsch <crydsch@lph.zone>
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.

None yet

1 participant