dotnet / machinelearning

ML.NET is an open source and cross-platform machine learning framework for .NET.
https://dot.net/ml
MIT License
9.04k stars 1.88k forks source link

MKLImports PDB not included with packages #5805

Open ericstj opened 3 years ago

ericstj commented 3 years ago

All binaries we release should have corresponding symbols.

We seem to have a manual process for building MKLImports due to the Intel MKL SDK dependency: https://github.com/dotnet/machinelearning/blob/04dda55ab0902982b16309c8e151f13a53e9366d/docs/building/MlNetMklDeps/README.md#windows

It would seem that the packages produced manually are missing the PDBs: https://dev.azure.com/dnceng/public/_packaging?_a=package&feed=machinelearning-assets&package=MlNetMklDeps&protocolType=NuGet&version=0.0.0.9&view=overview

So that when we redistribute this binary it is missing debug information.

At a minimum we should include the PDBs and redistribute those in the same way we do other native code that we build. This won't impact our final package size, only the MlNetMklDeps transport package.

Ideally we should include the build of this component as an optional part of our regular build process, so that we aren't relying on a manual build process. We still might want to keep MlNetMklDeps around so that contributors don't need to install the Intel MKL sdk if they want to build the entire product.

minghan commented 3 years ago

Please consider source link too. See https://www.nuget.org/packages/Microsoft.SourceLink.GitHub and https://github.com/dotnet/sourcelink

Thanks!

ericstj commented 3 years ago

Yes, we do source link as part of the dotnet/arcade infrastructure that builds this repository, but that won't help for third party components like this one, unfortunately, where we are just linking Intel's prebuilt libraries into a smaller dll. Perhaps we could ask Intel to publish sources and publish source-link PDBs along with their SDK. I'm not sure of the history here: has Intel ever considered OSS for their MKL (Math Kernel Library) project?

ericstj commented 3 years ago

As part of fixing this we should also update the version of MKL we're using to latest (if possible) and ensure we use a consistent one across all platforms. Related to https://github.com/dotnet/machinelearning/issues/1073