[WIP] Java Native Remote Inference #36623
Conversation
|
Hi @jrmccluskey, would like to get your review. Thanks. |
There was a problem hiding this comment.
I like the bones here, although I think there needs to be a bit of work around the inputs being sequences of input types + having the outputs be (InputT, OutputT) tuples. Makes it easier to start from that point than to try and retrofit things once batching exists.
| import org.apache.beam.sdk.transforms.ParDo; | ||
| import org.apache.beam.sdk.values.TypeDescriptor; | ||
|
|
||
| public class Example { |
There was a problem hiding this comment.
I'd like to see this written as an integration test rather than just an example
|
|
||
| import java.util.stream.Collectors; | ||
|
|
||
| public class OpenAiModelHandler |
There was a problem hiding this comment.
Stylize this as OpenAIModelHandler (this goes for all of the other classes as well)
| return builder().setParameters(modelParameters).build(); | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
From the Python implementation (as well as the cross-language implementation in Java) we're generally trying to return input-output pairs to make it easier to process the results downstream.
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * License); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an AS IS BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ |
There was a problem hiding this comment.
The Apache license header needs to be at the top of every new file (this is what the RAT pre-commit check is)
2 participants
Base Implementation for Java Native Remote Inference
addresses #36253
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.