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

docs: Adding document and notebooks for ONNXModel #1164

Merged
merged 10 commits into from Aug 17, 2021

Conversation

memoryz
Copy link
Contributor

@memoryz memoryz commented Aug 14, 2021

No description provided.

@memoryz
Copy link
Contributor Author

memoryz commented Aug 14, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@memoryz memoryz changed the title doc: Adding document and notebooks for ONNXModel docs: Adding document and notebooks for ONNXModel Aug 14, 2021
@memoryz
Copy link
Contributor Author

memoryz commented Aug 14, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@codecov
Copy link

codecov bot commented Aug 14, 2021

Codecov Report

Merging #1164 (cb3a4c6) into master (1f9135f) will decrease coverage by 0.06%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1164      +/-   ##
==========================================
- Coverage   83.81%   83.75%   -0.07%     
==========================================
  Files         259      259              
  Lines       12413    12433      +20     
  Branches      632      630       -2     
==========================================
+ Hits        10404    10413       +9     
- Misses       2009     2020      +11     
Impacted Files Coverage Δ
.../scala/com/microsoft/ml/spark/onnx/ONNXModel.scala 11.58% <0.00%> (-0.97%) ⬇️
...com/microsoft/ml/spark/core/utils/AsyncUtils.scala 65.00% <0.00%> (-5.00%) ⬇️
...rosoft/ml/spark/stages/PartitionConsolidator.scala 93.61% <0.00%> (-2.13%) ⬇️
.../microsoft/ml/spark/cognitive/ComputerVision.scala 78.57% <0.00%> (-0.52%) ⬇️
...microsoft/ml/spark/cognitive/SpeechToTextSDK.scala 87.83% <0.00%> (+0.76%) ⬆️
...a/com/microsoft/ml/spark/io/http/HTTPClients.scala 73.33% <0.00%> (+16.66%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f9135f...cb3a4c6. Read the comment docs.

@memoryz memoryz marked this pull request as draft August 14, 2021 01:46
@memoryz
Copy link
Contributor Author

memoryz commented Aug 14, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@memoryz
Copy link
Contributor Author

memoryz commented Aug 15, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@memoryz memoryz marked this pull request as ready for review August 15, 2021 00:42
@memoryz
Copy link
Contributor Author

memoryz commented Aug 16, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

}
}

private def writeNestedSeqToStringBuffer(nestedSeq: Seq[_], size: Int): ArrayBuffer[String] = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you share a little details on why this stuff is necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method writeNestedSeqToStringBuffer, together with writeNestedSeqToBuffer, are used to replace the flattenNested method.

flattenNested flattens a nested Seq to a 1-d array, but is super slow due to repeated memory allocation.
writeNestedSeqToStringBuffer and writeNestedSeqToBuffer both allocates the memory buffer one time, and write the nested Seq to the buffer. I did some local testing, and they appear to be 5-10 times faster than flattenNested.

The reason I need two methods is because there is not a nio.Buffer class for string type, so I had to replace it with a ArrayBuffer[String].

docs/onnx.md Outdated Show resolved Hide resolved
@memoryz memoryz marked this pull request as draft August 16, 2021 23:14
@memoryz
Copy link
Contributor Author

memoryz commented Aug 17, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@memoryz memoryz marked this pull request as ready for review August 17, 2021 06:19
@mhamilton723 mhamilton723 merged commit 21d5ec8 into microsoft:master Aug 17, 2021
@memoryz memoryz deleted the jasowang/onnxdoc branch August 17, 2021 17:47
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.

None yet

3 participants