Skip to content

Support more types for HashEstimator #5104

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

Merged
merged 17 commits into from
May 19, 2020
Merged

Support more types for HashEstimator #5104

merged 17 commits into from
May 19, 2020

Conversation

wangyems
Copy link
Contributor

@wangyems wangyems commented May 7, 2020

-Verified compatibility(change required in ML.NET in this PR) between ORT(microsoft/onnxruntime#3827) and ML.NET
-Some changes made in a couple of 64-bit hash function because the existed implementation is not exactly the same as MurmurHash3_x86_32
-bump to ort1.3 pre-release and fix some test cases

@codecov
Copy link

codecov bot commented May 14, 2020

Codecov Report

Merging #5104 into master will decrease coverage by 0.04%.
The diff coverage is 97.61%.

@@            Coverage Diff             @@
##           master    #5104      +/-   ##
==========================================
- Coverage   75.64%   75.59%   -0.05%     
==========================================
  Files         990      990              
  Lines      179576   179222     -354     
  Branches    19308    19287      -21     
==========================================
- Hits       135844   135488     -356     
- Misses      38462    38471       +9     
+ Partials     5270     5263       -7     
Flag Coverage Δ
#Debug 75.59% <97.61%> (-0.05%) ⬇️
#production 71.49% <88.88%> (-0.05%) ⬇️
#test 88.86% <100.00%> (-0.04%) ⬇️
Impacted Files Coverage Δ
src/Microsoft.ML.Data/Transforms/Hashing.cs 78.41% <88.88%> (-2.97%) ⬇️
...icrosoft.ML.TestFramework/DataPipe/TestDataPipe.cs 91.99% <100.00%> (ø)
test/Microsoft.ML.Tests/OnnxConversionTest.cs 99.36% <100.00%> (-0.07%) ⬇️
test/Microsoft.ML.Tests/Transformers/HashTests.cs 100.00% <100.00%> (ø)
...soft.ML.Data/DataLoadSave/DataOperationsCatalog.cs 77.17% <0.00%> (-4.65%) ⬇️
...icrosoft.ML.AutoML/Experiment/SuggestedPipeline.cs 88.65% <0.00%> (-4.13%) ⬇️
...crosoft.ML.Data/Commands/CrossValidationCommand.cs 81.48% <0.00%> (-2.05%) ⬇️
...L.AutoML/TrainerExtensions/TrainerExtensionUtil.cs 85.15% <0.00%> (-1.75%) ⬇️
src/Microsoft.ML.Data/Commands/TrainCommand.cs 81.88% <0.00%> (-1.45%) ⬇️
... and 23 more

@wangyems wangyems marked this pull request as ready for review May 14, 2020 22:00
@wangyems wangyems requested a review from a team as a code owner May 14, 2020 22:00
@harishsk
Copy link
Contributor

Is this file still part of the change list? If so, can you please revert this?


Refers to: Directory.Build.props:2 in 792703d. [](commit_id = 792703d, deletion_comment = False)

@wangyems wangyems marked this pull request as draft May 19, 2020 18:44
@wangyems wangyems marked this pull request as ready for review May 19, 2020 19:03
@wangyems wangyems requested a review from harishsk May 19, 2020 19:03
Copy link
Contributor

@harishsk harishsk left a comment

Choose a reason for hiding this comment

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

:shipit:

@wangyems wangyems deleted the wangye/hash branch June 9, 2020 20:19
@ghost ghost locked as resolved and limited conversation to collaborators Mar 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants