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.

Sign up for GitHub

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

Jump to bottom

Torch sharp version updates and test fixes #6954

Merged
merged 3 commits into from
Jan 17, 2024
Merged

Torch sharp version updates and test fixes #6954

merged 3 commits into from
Jan 17, 2024

Conversation

michaelgsharp
Copy link
Member

This PR updates the version of TorchSharp to the latest version to take advantage of some of the bug fixes it has that have been reported to us by some of our users.

It also fixes some test issues (makes the memory intensive ones actually run sequentially, they weren't doing it correctly before), and fixes the test namespace in the new test projects.

@ghost ghost assigned michaelgsharp Jan 13, 2024
tarekgh
tarekgh reviewed Jan 16, 2024
View reviewed changes
test/Microsoft.ML.TorchSharp.Tests/Microsoft.ML.TorchSharp.Tests.csproj
@@ -3,6 +3,9 @@
<AssemblyName>Microsoft.ML.TorchSharp.Tests</AssemblyName>
<StrongNameKeyId>Test</StrongNameKeyId>

<!-- Remove once we have resolved the TorchSharp issue. -->
<ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch>None</ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch>
Copy link
Member

Choose a reason for hiding this comment

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

ResolveAssemblyWarnOrErrorOnTargetArchitectureMismatch

What this exactly does?

Copy link
Member

Choose a reason for hiding this comment

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

It's required to avoid MSB3270, but I thought this wasn't needed on the 8.0 SDK - I thought that warning was completely removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope. Its still required unless I am doing something wrong. Throws an error without that.

@ericstj
Copy link
Member

ericstj commented Jan 16, 2024

At least one of the code-coverage failures mentions torchsharp: https://dev.azure.com/dnceng-public/public/_build/results?buildId=528996&view=ms.vss-test-web.build-test-results-tab&runId=12474756&resultId=101332&paneView=debug

System.DllNotFoundException : Unable to load DLL 'LibTorchSharp' or one of its dependencies: The specified module could not be found. (0x8007007E)

Stack trace
   at TorchSharp.PInvoke.NativeMethods.THSNN_custom_module(String name, ForwardFunctionC forward, IntPtr& pBoxedModule)
   at TorchSharp.torch.nn.Module..ctor(String name)
   at TorchSharp.torch.nn.HookableModule`2..ctor(String name)
   at TorchSharp.torch.nn.Module`3..ctor(String name)
   at Microsoft.ML.TorchSharp.NasBert.Models.BaseModel..ctor(NasBertOptions options) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\NasBert\Models\BaseModel.cs:line 25
   at Microsoft.ML.TorchSharp.NasBert.Models.NasBertModel..ctor(NasBertOptions options, Int32 padIndex, Int32 symbolsCount) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\NasBert\Models\NasBertModel.cs:line 18
   at Microsoft.ML.TorchSharp.NasBert.Models.ModelForPrediction..ctor(NasBertOptions options, Int32 padIndex, Int32 symbolsCount, Int32 numClasses) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\NasBert\Models\ModelPrediction.cs:line 17
   at Microsoft.ML.TorchSharp.NasBert.NasBertTrainer`2.NasBertTrainerBase.CreateModule(IChannel ch, IDataView input) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\NasBert\NasBertTrainer.cs:line 210
   at Microsoft.ML.TorchSharp.TorchSharpBaseTrainer`2.TrainerBase..ctor(TorchSharpBaseTrainer`2 parent, IChannel ch, IDataView input, String modelUrl) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\TorchSharpBaseTrainer.cs:line 155
   at Microsoft.ML.TorchSharp.NasBert.NasBertTrainer`2.NasBertTrainerBase..ctor(TorchSharpBaseTrainer`2 parent, IChannel ch, IDataView input, String modelUrl) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\NasBert\NasBertTrainer.cs:line 185
   at Microsoft.ML.TorchSharp.NasBert.TextClassificationTrainer.Trainer..ctor(TorchSharpBaseTrainer`2 parent, IChannel ch, IDataView input) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\NasBert\TextClassificationTrainer.cs:line 116
   at Microsoft.ML.TorchSharp.NasBert.TextClassificationTrainer.CreateTrainer(TorchSharpBaseTrainer`2 parent, IChannel ch, IDataView input) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\NasBert\TextClassificationTrainer.cs:line 104
   at Microsoft.ML.TorchSharp.TorchSharpBaseTrainer`2.Fit(IDataView input) in D:\a\_work\1\s\src\Microsoft.ML.TorchSharp\TorchSharpBaseTrainer.cs:line 107
   at Microsoft.ML.Data.EstimatorChain`1.Fit(IDataView input) in D:\a\_work\1\s\src\Microsoft.ML.Data\DataLoadSave\EstimatorChain.cs:line 68
   at Microsoft.ML.RunTests.TestDataPipeBase.TestEstimatorCore(IEstimator`1 estimator, IDataView validFitInput, IDataView validTransformInput, IDataView invalidInput, IDataView validForFitNotValidForTransformInput, Boolean shouldDispose) in D:\a\_work\1\s\test\Microsoft.ML.TestFramework\DataPipe\TestDataPipeBase.cs:line 80
   at Microsoft.ML.TorchSharp.Tests.TextClassificationTests.TestSingleSentence3Classes() in D:\a\_work\1\s\test\Microsoft.ML.TorchSharp.Tests\TextClassificationTests.cs:line 239

@codecov Codecov
Copy link

codecov bot commented Jan 17, 2024

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (d0d8569) 68.80% compared to head (b8f4b4a) 68.80%.
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #6954      +/-   ##
==========================================
- Coverage   68.80%   68.80%   -0.01%     
==========================================
  Files        1249     1249              
  Lines      249644   249686      +42     
  Branches    25481    25485       +4     
==========================================
+ Hits       171778   171800      +22     
- Misses      71275    71292      +17     
- Partials     6591     6594       +3
Flag Coverage Δ
Debug 68.80% <0.00%> (-0.01%) ⬇️
production 63.27% <0.00%> (-0.01%) ⬇️
test 88.41% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...t/Microsoft.ML.TensorFlow.Tests/TensorflowTests.cs 91.64% <ø> (-0.08%) ⬇️
test/Microsoft.ML.TorchSharp.Tests/NerTests.cs 67.02% <ø> (ø)
...rosoft.ML.TorchSharp.Tests/ObjectDetectionTests.cs 100.00% <ø> (ø)
test/Microsoft.ML.TorchSharp.Tests/QATests.cs 40.50% <ø> (ø)
...oft.ML.TorchSharp.Tests/TextClassificationTests.cs 85.17% <0.00%> (ø)
....TorchSharp/AutoFormerV2/ObjectDetectionTrainer.cs 86.57% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs 78.30% <0.00%> (ø)
...c/Microsoft.ML.TorchSharp/TorchSharpBaseTrainer.cs 84.98% <0.00%> (ø)

... and 13 files with indirect coverage changes

@michaelgsharp michaelgsharp merged commit b824ed2 into dotnet:main Jan 17, 2024
25 checks passed
@michaelgsharp
Copy link
Member Author

/backport to release/3.0

@github-actions GitHub Actions
Copy link

Started backporting to release/3.0: https://github.com/dotnet/machinelearning/actions/runs/7550850328

@github-actions github-actions bot mentioned this pull request Jan 17, 2024
Merged
@michaelgsharp michaelgsharp mentioned this pull request Jan 17, 2024
Open
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2024
@michaelgsharp michaelgsharp deleted the torch-version-update branch November 18, 2024 20:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tarekgh tarekgh tarekgh approved these changes

@ericstj ericstj ericstj approved these changes

Assignees

@michaelgsharp michaelgsharp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@michaelgsharp @ericstj @tarekgh