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

Ensure Batchifier returns compatible arrays for hybrid engines like OnnxRuntime #3447

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

david-sitsky
Copy link
Contributor

Addresses issue raised from https://deepjavalibrary.slack.com/archives/C01AURG857U/p1724804411722599.

Essentially batchifying / unbatchifying OrtNDArrays produce PtNDArrays, and this causes downstream issues, particularly in OrtSymbolBlock:114:

            OrtSession.Result results = session.run(container);
            NDList ret = evaluateOutput(results);
            ret.attach(inputs.head().getManager());

Because of the batchify call during Translator processInput() call, the initial OrtNDArrays end up as PtNDArrays, and then the ret.attach call incorrectly sets the ret NDList's main (and alternate manager) both to PtNDManager. This inhibits a lot of post-processing code from working as the returned NDArrays have the wrong manager set.

Maybe there is a more elegant way of fixing this within OrtSymbolBlock, but I couldn't see any easy way of passing in the Predict NDManager in.

@david-sitsky david-sitsky requested review from zachgk, frankfliu and a team as code owners August 30, 2024 05:24
@david-sitsky
Copy link
Contributor Author

I'm not really a fan of this change as I think it will introduce too much memory copying. Maybe there is a better way?

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.

1 participant