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

Improve ease of use of acquisition function reducer #91

Closed
joelberkeley-secondmind opened this issue Dec 3, 2020 · 2 comments
Closed
Labels
enhancement New feature or request

Comments

@joelberkeley-secondmind
Copy link
Member

Describe the feature you'd like
As a user, I want to be able to compose acquisition function builders with minimal effort and minimal code.

Is your feature request related to a problem? Please describe.
At the moment, one needs to define a new class to create a custom reducer, but since it only needs one function implementing, users should be able to specify just the new function

@joelberkeley-secondmind joelberkeley-secondmind added the enhancement New feature or request label Dec 3, 2020
@joelberkeley-secondmind
Copy link
Member Author

joelberkeley-secondmind commented Dec 23, 2020

Options:

  1. make Reducer a function (tf.Tensor -> tf.Tensor) -> Sequence[AcquisitionFunctionBuilder] -> AcquisitionFunctionBuilder
  2. add a classmethod from_fn: (tf.Tensor -> tf.Tensor) -> Reducer, though this would only make sense if we can do it in such a way that subclasses don't have access to the method ... i.e. make it a module-level function. However, this is the same as 1.

@uri-granta
Copy link
Collaborator

Don't think this is worthwhile, mainly because a functional constructor makes it more difficult (maybe impossible?) to specify generic typing. E.g. Sum = Reducer(tf.add_n) can't specify the fact that Sum is generic in the type of supported models (i.e. if all the builders support some ProbabilisticModelType then so does Sum). Furthermore, we've seen very little use of reducers since this ticket was raised, and being forced to define a named subclass is arguably more Pythonic anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants