Skip to content

Conversation

@RandySheriffH
Copy link
Contributor

@RandySheriffH RandySheriffH commented Feb 8, 2021

Per team's decision, deprecate OMP from packaging to prepare for coming release:

  1. For previous openmp build, remove --use_openmp, so thread pool will become default;
  2. For previous non-openmp build, add --use_openmp and rename the package to indicate the inclusion.
  3. Add a mac build with openmp enabled.

@RandySheriffH RandySheriffH requested a review from a team as a code owner February 8, 2021 18:33
@RandySheriffH RandySheriffH requested review from a team, pranavsharma and snnn February 8, 2021 18:42
@snnn
Copy link
Contributor

snnn commented Feb 8, 2021

Please also add a package for MacOS openmp. And, in the noopenmp case, please remove the commands for installing openmp from brew.

@snnn
Copy link
Contributor

snnn commented Feb 12, 2021

As we talked offline:

In total we should have 5 different nuget variants:

  1. CPU with openmp
  2. CPU without openmp
  3. GPU
  4. CPU without contribops and without openmp
  5. Windows AI

And we have 8 packaging piplines. Below are my suggestions:

  1. Node.js binding : Need to remove openmp
  2. Nuget GPU  (OnnxRuntime-Github): no need to change
  3. Nuget WindowsAI: no need to change
  4. Python packaging: update the package names and add MacOS
  5. Python Packaging  (Training) : no need to change
  6. Zip-Nuget-Java Packaging: rename the pipeline name and packages to openmp. And move the GPU part to NoOpenMP pipeline.
  7. Zip-Nuget-Java Packaging  NoOpenMP: add GPU part from the noopenmp pipeline.
  8. Zip-Nuget-Java Packaging  NoContribOps: no need to change.

Please correct me if I was wrong.

And I suggest you split this PR to 3: nodejs/python/nuget. So that we can validate them separately.

And, BTW, in the master branch the noopenmp nuget pipeline still has a tiny problem. I have a fix in #6664. The others are green now.

@RandySheriffH RandySheriffH changed the title Deprecate OMP from packaging pipeline Deprecate OMP from Python package Feb 12, 2021
@snnn
Copy link
Contributor

snnn commented Feb 13, 2021

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants