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

PI improvements #6229

Merged
merged 8 commits into from
Aug 22, 2018
Merged

PI improvements #6229

merged 8 commits into from
Aug 22, 2018

Conversation

raver119
Copy link
Contributor

@raver119 raver119 commented Aug 21, 2018

WIP; DO NOT MERGE;

This PR adds:

  • additional ParallelInference mode: INPLACE
  • OutputAdapter concept for workspace-based conversion of Model output to custom Java entities

Copy link
Contributor

@AlexDBlack AlexDBlack left a comment

Choose a reason for hiding this comment

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

Minor polish/docs, otherwise LGTM 👍

import java.util.concurrent.locks.ReentrantReadWriteLock;

/**
* This ParallelInference implementation provides inference functionality without launching additional threads, so inference happens in the calling thread
Copy link
Contributor

Choose a reason for hiding this comment

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

More javadoc - limitations, when to use.
Also how to build one (i.e., "use PI builder configured with X, not this class") - that might not be obvious to users at first glance (it wasn't for me)

}


public static class ModelSelector {
Copy link
Contributor

Choose a reason for hiding this comment

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

Make protected/private, unless there's a reason for it to be public?

BATCHED,

/**
* Inference will applied in the calling thread instead of workers.
Copy link
Contributor

Choose a reason for hiding this comment

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

More javadoc - limitations, etc

ROUND_ROBIN,

/**
* in this mode we'll be
Copy link
Contributor

Choose a reason for hiding this comment

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

javadoc

@raver119 raver119 merged commit c8dba00 into master Aug 22, 2018
@raver119 raver119 deleted the r119_pi branch August 22, 2018 02:46
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.

2 participants