-
Notifications
You must be signed in to change notification settings - Fork 47
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
Fix batch recommendation #348
Conversation
6c1eee5
to
776aef1
Compare
776aef1
to
63f511e
Compare
67cc35e
to
25ccfb8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure whether I fully understand some of the changes made here. In general this looks good, but I'd appreciate some details since I do not know if I understood everything correctly
1296e25
to
f3c2ac7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since all of my question are just minor/my understanding, feel free to resolve as you see fit :)
ec950da
to
4dac89d
Compare
When the non-GP surrogates where added, they were not designed with batch recommendations in mind. Now that batch recommendations can be generally requested, surrogate models providing only marginal posterior information yield unusable results for batch sizes larger than one.
This PR fixes the issue by:
RandomForestSurrogate
produce a properEnsemblePosterior
that is capable of expressing covariance structure, which is the basic requirement for batch prediction