Add NameEntityRecognition and Q&A deep learning tasks. - #6760

Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom
michaelgsharp:ner
Jul 24, 2023
Merged

Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom
michaelgsharp:ner

Conversation

@michaelgsharp

@michaelgsharpmichaelgsharp commented Jul 10, 2023

Copy link
Copy Markdown
Contributor

This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.

The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.

@michaelgsharp

michaelgsharp commented Jul 10, 2023

Copy link
Copy Markdown
ContributorAuthor

Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.

JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs Outdated
JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
internal sealed class ModelForPrediction : NasBertModel

@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NERInferenceModel?

@michaelgsharpmichaelgsharpJul 17, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?

JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT reviewed Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs Outdated
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs Outdated
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs Outdated
ericstj
ericstj reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs Outdated
ericstj
ericstj reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs Outdated
zewditu
zewditu reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu reviewed Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu reviewed Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu reviewed Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj reviewed Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs Outdated
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs Outdated
ericstj
ericstj reviewed Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs Outdated
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

for (var i = 0; i < srcTokens.size(0); ++i)
{
var srcTokenArray = srcTokens[i].ToArray<int>();

@ericstjericstjJul 11, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?

@michaelgsharpmichaelgsharpJul 17, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?

@NiklasGustafssonNiklasGustafssonJul 18, 2023•
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.

@michaelgsharpmichaelgsharpJul 18, 2023

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?

@NiklasGustafssonNiklasGustafssonJul 18, 2023•
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.

publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}

LittleLittleCloud
LittleLittleCloud reviewed Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud reviewed Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot mentioned this pull request Jul 18, 2023
Open
5 tasks
@codecov

codecovBot commented Jul 18, 2023•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.
The diff coverage is 78.00%.

Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️

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

Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more

... and 36 files with indirect coverage changes

ericstj
ericstj approved these changes Jul 19, 2023
View reviewed changes

@ericstjericstj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass

@michaelgsharp
michaelgsharp merged commit 65c7ca9 into dotnet:mainJul 24, 2023
@ghostghost locked as resolved and limited conversation to collaborators Aug 24, 2023
@michaelgsharp
michaelgsharp deleted the ner branch November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla
+2 more reviewers
@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Add NameEntityRecognition and Q&A deep learning tasks. - #6760
Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Jul 24, 2023
Merged
Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Conversation
@michaelgsharp
@michaelgsharpmichaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

Contributor
This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.
The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.
@michaelgsharp
michaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

ContributorAuthor
Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
Outdated

JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
 internal sealed class ModelForPrediction : NasBertModel
@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NERInferenceModel?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

 for (var i = 0; i < srcTokens.size(0); ++i)
 {
 var srcTokenArray = srcTokens[i].ToArray<int>();
@ericstjericstjJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.
@michaelgsharpmichaelgsharpJul 18, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.
publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 18, 2023

Open

5 tasks
@codecov
codecovBot
commented
Jul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.

The diff coverage is 78.00%.
Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112 
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more
... and 36 files with indirect coverage changes
ericstj
ericstj
approved these changes
Jul 19, 2023
View reviewed changes
@ericstjericstj
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass
@michaelgsharp
michaelgsharp
merged commit 65c7ca9
into
dotnet:mainJul 24, 2023
@ghostghost
locked as resolved and limited conversation to collaborators
Aug 24, 2023
@michaelgsharp
michaelgsharp
deleted the

ner

branch
November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla

+2 more reviewers

@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add NameEntityRecognition and Q&A deep learning tasks. - #6760
Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Jul 24, 2023
Merged
Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Conversation
@michaelgsharp
@michaelgsharpmichaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

Contributor
This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.
The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.
@michaelgsharp
michaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

ContributorAuthor
Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
Outdated

JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
 internal sealed class ModelForPrediction : NasBertModel
@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NERInferenceModel?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

 for (var i = 0; i < srcTokens.size(0); ++i)
 {
 var srcTokenArray = srcTokens[i].ToArray<int>();
@ericstjericstjJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.
@michaelgsharpmichaelgsharpJul 18, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.
publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 18, 2023

Open

5 tasks
@codecov
codecovBot
commented
Jul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.

The diff coverage is 78.00%.
Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112 
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more
... and 36 files with indirect coverage changes
ericstj
ericstj
approved these changes
Jul 19, 2023
View reviewed changes
@ericstjericstj
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass
@michaelgsharp
michaelgsharp
merged commit 65c7ca9
into
dotnet:mainJul 24, 2023
@ghostghost
locked as resolved and limited conversation to collaborators
Aug 24, 2023
@michaelgsharp
michaelgsharp
deleted the

ner

branch
November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla

+2 more reviewers

@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add NameEntityRecognition and Q&A deep learning tasks. - #6760
Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Jul 24, 2023
Merged
Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Conversation
@michaelgsharp
@michaelgsharpmichaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

Contributor
This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.
The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.
@michaelgsharp
michaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

ContributorAuthor
Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
Outdated

JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
 internal sealed class ModelForPrediction : NasBertModel
@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NERInferenceModel?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

 for (var i = 0; i < srcTokens.size(0); ++i)
 {
 var srcTokenArray = srcTokens[i].ToArray<int>();
@ericstjericstjJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.
@michaelgsharpmichaelgsharpJul 18, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.
publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 18, 2023

Open

5 tasks
@codecov
codecovBot
commented
Jul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.

The diff coverage is 78.00%.
Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112 
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more
... and 36 files with indirect coverage changes
ericstj
ericstj
approved these changes
Jul 19, 2023
View reviewed changes
@ericstjericstj
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass
@michaelgsharp
michaelgsharp
merged commit 65c7ca9
into
dotnet:mainJul 24, 2023
@ghostghost
locked as resolved and limited conversation to collaborators
Aug 24, 2023
@michaelgsharp
michaelgsharp
deleted the

ner

branch
November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla

+2 more reviewers

@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Add NameEntityRecognition and Q&A deep learning tasks. - #6760
Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Jul 24, 2023
Merged
Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Conversation
@michaelgsharp
@michaelgsharpmichaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

Contributor
This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.
The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.
@michaelgsharp
michaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

ContributorAuthor
Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
Outdated

JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
 internal sealed class ModelForPrediction : NasBertModel
@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NERInferenceModel?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

 for (var i = 0; i < srcTokens.size(0); ++i)
 {
 var srcTokenArray = srcTokens[i].ToArray<int>();
@ericstjericstjJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.
@michaelgsharpmichaelgsharpJul 18, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.
publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 18, 2023

Open

5 tasks
@codecov
codecovBot
commented
Jul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.

The diff coverage is 78.00%.
Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112 
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more
... and 36 files with indirect coverage changes
ericstj
ericstj
approved these changes
Jul 19, 2023
View reviewed changes
@ericstjericstj
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass
@michaelgsharp
michaelgsharp
merged commit 65c7ca9
into
dotnet:mainJul 24, 2023
@ghostghost
locked as resolved and limited conversation to collaborators
Aug 24, 2023
@michaelgsharp
michaelgsharp
deleted the

ner

branch
November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla

+2 more reviewers

@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add NameEntityRecognition and Q&A deep learning tasks. - #6760
Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Jul 24, 2023
Merged
Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Conversation
@michaelgsharp
@michaelgsharpmichaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

Contributor
This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.
The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.
@michaelgsharp
michaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

ContributorAuthor
Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
Outdated

JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
 internal sealed class ModelForPrediction : NasBertModel
@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NERInferenceModel?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

 for (var i = 0; i < srcTokens.size(0); ++i)
 {
 var srcTokenArray = srcTokens[i].ToArray<int>();
@ericstjericstjJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.
@michaelgsharpmichaelgsharpJul 18, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.
publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 18, 2023

Open

5 tasks
@codecov
codecovBot
commented
Jul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.

The diff coverage is 78.00%.
Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112 
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more
... and 36 files with indirect coverage changes
ericstj
ericstj
approved these changes
Jul 19, 2023
View reviewed changes
@ericstjericstj
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass
@michaelgsharp
michaelgsharp
merged commit 65c7ca9
into
dotnet:mainJul 24, 2023
@ghostghost
locked as resolved and limited conversation to collaborators
Aug 24, 2023
@michaelgsharp
michaelgsharp
deleted the

ner

branch
November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla

+2 more reviewers

@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add NameEntityRecognition and Q&A deep learning tasks. - #6760
Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Jul 24, 2023
Merged
Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Conversation
@michaelgsharp
@michaelgsharpmichaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

Contributor
This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.
The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.
@michaelgsharp
michaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

ContributorAuthor
Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
Outdated

JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
 internal sealed class ModelForPrediction : NasBertModel
@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NERInferenceModel?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

 for (var i = 0; i < srcTokens.size(0); ++i)
 {
 var srcTokenArray = srcTokens[i].ToArray<int>();
@ericstjericstjJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.
@michaelgsharpmichaelgsharpJul 18, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.
publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 18, 2023

Open

5 tasks
@codecov
codecovBot
commented
Jul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.

The diff coverage is 78.00%.
Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112 
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more
... and 36 files with indirect coverage changes
ericstj
ericstj
approved these changes
Jul 19, 2023
View reviewed changes
@ericstjericstj
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass
@michaelgsharp
michaelgsharp
merged commit 65c7ca9
into
dotnet:mainJul 24, 2023
@ghostghost
locked as resolved and limited conversation to collaborators
Aug 24, 2023
@michaelgsharp
michaelgsharp
deleted the

ner

branch
November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla

+2 more reviewers

@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Add NameEntityRecognition and Q&A deep learning tasks. - #6760
Merged
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Jul 24, 2023
Merged
Add NameEntityRecognition and Q&A deep learning tasks.#6760
michaelgsharp merged 16 commits into
dotnet:mainfrom 
michaelgsharp:ner
Conversation
@michaelgsharp
@michaelgsharpmichaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

Contributor
This PR adds in 2 new deep learning scenarios, Name Entity Recognition and Q&A.
The main files to focus on are NerTrainer.cs and Roberta/QATrainer.cs. Most of the rest are either part of the deep learning model itself or internal implementations of things I had to copy over from runtime for them to work on netstandard.
@michaelgsharp
michaelgsharp
commented
Jul 10, 2023

Copy link


Copy Markdown

ContributorAuthor
Q&A currently has a runtime error I am working on resolving so the builds will fail for now. Getting the PR up so reviews can start while I finish debugging.
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelForNer.cs
Outdated

JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/ModelPrediction.cs

namespace Microsoft.ML.TorchSharp.NasBert.Models
{
 internal sealed class ModelForPrediction : NasBertModel
@JakeRadMSFTJakeRadMSFTJul 10, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

NERInferenceModel?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

This isn't for NER. Its for SentenceSimilarity and TextClassification. How about TextModel? TextModelForPrediction? Thoughts?
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/Models/TransformerEncoder.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
JakeRadMSFT
JakeRadMSFT
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Utils/Range.cs
Outdated

ericstj
ericstj
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/NasBert/NerTrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
zewditu
zewditu
reviewed
Jul 10, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/QATests.cs
ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

ericstj
ericstj
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs
Outdated

Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/Models/RobertaModel.cs

 for (var i = 0; i < srcTokens.size(0); ++i)
 {
 var srcTokenArray = srcTokens[i].ToArray<int>();
@ericstjericstjJul 11, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I wonder if we could use the TensorAccessor exposed by the data method and avoid this array?
@michaelgsharpmichaelgsharpJul 17, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Looking into it it doesn't look like the TensorAccessor exposes enough to be able to do that. @NiklasGustafsson do you know if that is correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

@michaelgsharp , I'm not sure which 'that' you're referring to. TensorAccessor implements IEnumerable.
@michaelgsharpmichaelgsharpJul 18, 2023

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

I know the TensorAccessor has direct access to the underlying memory, but it doesn't expose the underlying memory directly, correct?
@NiklasGustafssonNiklasGustafssonJul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Choose a reason for hiding this comment

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

Indexing into the accessor will access the underlying native memory directly, both reading and writing, while ToArray() will make a copy.
publicTthis[paramslong[] indices]{get{longindex=0;if(indices.Length==1){index=indices[0];validate(index);
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(index,_tensor)];}}else{
unsafe {T*ptr=(T*)_tensor_data_ptr;returnptr[TranslateIndex(indices,_tensor)];}}}
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadtest/Microsoft.ML.Tests/NerTests.cs
LittleLittleCloud
LittleLittleCloud
reviewed
Jul 11, 2023
View reviewed changes
Comment threadsrc/Microsoft.ML.TorchSharp/Roberta/QATrainer.cs
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 18, 2023

Open

5 tasks
@codecov
codecovBot
commented
Jul 18, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #6760 (407964d) into main (8858ab6) will increase coverage by 0.10%.

The diff coverage is 78.00%.
Additional details and impacted files
@@ Coverage Diff @@## main #6760 +/- ##
==========================================
+ Coverage 68.89% 68.99% +0.10% 
==========================================
Files 1216 1237 +21 Lines 250915 252836 +1921 Branches 26259 26445 +186 ==========================================
+ Hits 172857 174450 +1593 - Misses 71238 71454 +216 - Partials 6820 6932 +112 
FlagCoverage Δ
Debug68.99% <78.00%> (+0.10%)⬆️
production63.56% <80.16%> (+0.17%)⬆️
test88.83% <61.17%> (-0.08%)⬇️
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted FilesCoverage Δ
src/Microsoft.ML.Tokenizers/Model/BPE.cs70.68% <0.00%> (-0.87%)⬇️
...Microsoft.ML.TorchSharp/NasBert/Models/BaseHead.cs100.00% <ø> (+100.00%)⬆️
...icrosoft.ML.TorchSharp/NasBert/Models/BaseModel.cs60.00% <0.00%> (-6.67%)⬇️
...oft.ML.TorchSharp/NasBert/Models/PredictionHead.cs100.00% <ø> (ø)
src/Microsoft.ML.TorchSharp/Utils/MethodTable.cs0.00% <0.00%> (ø)
src/Microsoft.ML.TorchSharp/Utils/Range.cs31.57% <ø> (ø)
...soft.ML.TestFramework/DataPipe/TestDataPipeBase.cs73.39% <0.00%> (-0.10%)⬇️
test/Microsoft.ML.Tests/QATests.cs40.50% <40.50%> (ø)
...osoft.ML.TorchSharp/NasBert/Models/NasBertModel.cs86.11% <50.00%> (-3.48%)⬇️
...oft.ML.TorchSharp/Roberta/Models/RobertaEncoder.cs50.00% <50.00%> (ø)
... and 32 more
... and 36 files with indirect coverage changes
ericstj
ericstj
approved these changes
Jul 19, 2023
View reviewed changes
@ericstjericstj
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks for resolving the feedback around GetSubArray - I'd still like for @LittleLittleCloud, @zewditu, or @JakeRadMSFT to give a pass
@michaelgsharp
michaelgsharp
merged commit 65c7ca9
into
dotnet:mainJul 24, 2023
@ghostghost
locked as resolved and limited conversation to collaborators
Aug 24, 2023
@michaelgsharp
michaelgsharp
deleted the

ner

branch
November 18, 2024 20:04
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@JakeRadMSFTJakeRadMSFTJakeRadMSFT left review comments
@ericstjericstjericstj approved these changes
@LittleLittleCloudLittleLittleCloudLittleLittleCloud approved these changes
@tannergoodingtannergoodingAwaiting requested review from tannergooding
@luisquintanillaluisquintanillaAwaiting requested review from luisquintanilla

+2 more reviewers

@NiklasGustafssonNiklasGustafssonNiklasGustafsson left review comments
@zewdituzewdituzewditu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@michaelgsharpmichaelgsharp

Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

6 participants

@michaelgsharp@NiklasGustafsson@ericstj@LittleLittleCloud@JakeRadMSFT@zewditu