Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss
, '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

Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss
, '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

Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss
, '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

Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss
, '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

Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss
, '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

Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss
, '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

Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss
, '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

Onedal algorithms backed by nuget packages - #6521

Merged
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget
Dec 21, 2022
Merged

Onedal algorithms backed by nuget packages#6521
michaelgsharp merged 66 commits into
dotnet:mainfrom
rgesteve:onedal_with_nuget

Conversation

@rgesteve

Copy link
Copy Markdown
Contributor

Supercedes #6373 and #6364 allowing for full build of functionality without extra packages (they're downloaded by build.sh). Also includes sample to illustrate use.

We are excited to review your PR.

So we can do the best job, please check:

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

@rgesteve@michaelgsharp -- I gave a local test of this and was able to reproduce the problem. It looks like the error is due to missing onedal_core.1.dll which is a dependency of OneDalNative.dll. You can see this if you enable loader snaps in the native debugger. The .NET runtime can't distinguish between a failure from the library itself vs some dependency as the failure happens inside the call to loadlibrary and the OS doesn't distinguish.

var currentDir = AppContext.BaseDirectory;
Output.WriteLine($"**** Running from directory {currentDir}.");

var dllDir = AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES").ToString();

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.

This doesn't work on .NETFramework, but I image you've just added this for debugging purposes. Make sure to remove it.

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.

Will do, this whole test is there on a very temporary basis.

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.

Commit #e2a60945 removes the test.

@ericstj

ericstj commented Dec 19, 2022

Copy link
Copy Markdown
Member

I tried to manually workaround the missing onedal_core.1.dll by finding it in the inteldal.redist.win-x64 nuget package and manually copying it over, then the test hit errors for other missing dlls:

Intel oneDAL FATAL ERROR: Cannot find/load library onedal_thread.1.dll.
Intel oneDAL FATAL ERROR: Cannot find/load library onedal_sequential.1.dll.
Intel oneDAL FATAL ERROR: Cannot load neither onedal_thread.1.dll nor onedal_sequential.1.dll

After this, the test failed fast and terminated the process. I couldn't find those dlls to copy over so it seems blocked.
Update: those dlls were present, I actually copied them over along with onedal_core.1.dll but for some reason the OS loader isn't probing next to onedal_core.1.dll it's only probing next to the application dotnet.exe (in this case) and other machine wide paths.

What's interesting is that this is different than the load for OneDalNative.dll > onedal_core.1.dll. In that case the OS considers the directory for OneDalNative.dll when looking for onedal_core.1.dll. This load is happening as a result of the import table in OneDalNative.dll, so it makes sense that the OS would look next to it. I'm not sure what is causing the load of onedal_thread.1.dll -- I don't see that in the import table of onedal_core.1.dll -- is this being loaded manually through a call in onedal_core.1.dll? If so, it could be that the flags passed are not permitting search next to the executing binary. I guess that would make sense for a normal call to load since it doesn't "know" that the load would be coming from a library that's not located in the application's directory. I think the fix here would be to make those loads in the onedal libraries explicitly consider the directory of the onedal_core.1.dll library when doing further loads.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Thank you Eric! Loading OneDalNative successfully and failing to load the dependencies was my observation as well in Linux:

 193059:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/.dotnet/shared/Microsoft.NETCore.App/6.0.9/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193060:[pid 21092] openat(AT_FDCWD, "/data/projects/machinelearning/artifacts/bin/Microsoft.ML.Tests/Debug/net6.0/libOneDalNative.so", O_RDONLY|O_CLOEXEC) = 415
193069:[pid 21092] openat(AT_FDCWD, "/etc/ld.so.cache", O_RDONLY|O_CLOEXEC) = 415
193073:[pid 21092] openat(AT_FDCWD, "/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193074:[pid 21092] openat(AT_FDCWD, "/usr/lib/x86_64-linux-gnu/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)
193075:[pid 21092] openat(AT_FDCWD, "/lib/libonedal_core.so.1.1", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory)

Like you said, the loader doesn't even bother trying to find the dependencies in directories other than the system-wide ldconfig standard ones. ldd does show libonedal_{core,thread} as dependencies of OneDalCore, but your suggestion of the library dynamically loading its dependencies makes a lot of sense, especially in light that, pointing LD_LIBRARY_PATH to the exact same directory, with the exact same contents that the one the workload should be search works. The last commit includes the dependencies in the nupkg for distribution. I don't expect that this will change the loader issue much, but at least it'll allow people to try explictitly pointing LD_LIBRARY_PATH/PATH to where the dependencies are in their filesystem.

In the meanwhile will work with the oneDAL team to see if we can figure out what's going on.

@ericstj

Copy link
Copy Markdown
Member

It might be worthwhile following up with Interop folks like @AaronRobinsonMSFT and @elinor-fung to understand if it is expected that the runtime doesn’t set this native probing path itself. It could be something odd about how ML.Net runs tests.

@rgesteve

Copy link
Copy Markdown
ContributorAuthor

Looks like commit 35a18a1 addresses this for Linux, by specifying in the RPATH that dependencies should be searched for in the same directory OneDalNative is found. Trying to figure out what the equivalent command line options are for the VC compiler

@michaelgsharpmichaelgsharp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit:

@michaelgsharp
michaelgsharp merged commit 0880a90 into dotnet:mainDec 21, 2022
@ghostghost locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@rgesteve@ericstj@michaelgsharp@Alexsandruss