Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

onnx optimizer init #2

Merged
merged 14 commits into from
Sep 7, 2020
Merged

onnx optimizer init #2

merged 14 commits into from
Sep 7, 2020

Conversation

daquexian
Copy link
Member

@daquexian daquexian commented Aug 23, 2020

Related issue: #1

This PR contains the separate onnx optimizer library, in detail:

  1. onnx optimizer codes and tests (optimizer_test.py) are copied from onnx/onnx to onnx/optimizer with minimal changes to make the build successful.
  2. cmake codes and setup.py are added to build and install the python package and c++ library

An onnxoptimizer (not sure whether it is a good name) package will be installed after running pip3 install -e ..
pytest passes.

@daquexian
Copy link
Member Author

After discussion with @linkerzhang, I renamed onnx_opt to onnxoptimizer

@linkerzhang
Copy link
Member

@daquexian thank you very much for making the change. are you going to setup CI for this repo please?

@daquexian
Copy link
Member Author

@linkerzhang Thanks for your advice!

I have copied the azure pipeline ci yaml files from onnx/onnx repo and made some necessary changes for onnx/optimizer

Could you please set it up so that we can see whether the ci passes? Thanks!

@linkerzhang
Copy link
Member

@linkerzhang Thanks for your advice!

I have copied the azure pipeline ci yaml files from onnx/onnx repo and made some necessary changes for onnx/optimizer

Could you please set it up so that we can see whether the ci passes? Thanks!

I guess I don't have such access neither right now.
@prasanthpul can you offer @daquexian the permission to enable CI for optimizer repo please?

@prasanthpul
Copy link
Member

Can you specify what you are looking for? Azure Pipelines is enabled for all repos under ONNX. I would also recommend using GitHub Actions

@daquexian
Copy link
Member Author

Can you specify what you are looking for? Azure Pipelines is enabled for all repos under ONNX. I would also recommend using GitHub Actions

I think a new pipeline needs to be added.

The step may be:

Click the "New Pipeline"

image

Select the correct repo, and select the "Existing Azure Pipelines YAML file"

image

@daquexian
Copy link
Member Author

@prasanthpul I just noticed that there is no pipeline yaml in onnx/optimizer repo now since this PR has not been merged. So your help will be needed after this PR is merged :)

@linkerzhang I will setup and test the ci in my own repo (daquexian/optimizer) for now

@daquexian
Copy link
Member Author

@linkerzhang linkerzhang merged commit 0299379 into onnx:master Sep 7, 2020
@askhade
Copy link

askhade commented Sep 10, 2020

@daquexian : Please prefer github actions for CIs. Azure pipelines and github actions are very similar and underneath they are based on same stack I believe... With github actions there is 1 layer less to interact. If you have a working azure pipeline CI then moving it to github actions should be trivial.

@daquexian
Copy link
Member Author

@daquexian : Please prefer github actions for CIs. Azure pipelines and github actions are very similar and underneath they are based on same stack I believe... With github actions there is 1 layer less to interact. If you have a working azure pipeline CI then moving it to github actions should be trivial.

I'll switch to github actions when I have time

@daquexian
Copy link
Member Author

daquexian commented Sep 13, 2020

@daquexian : Please prefer github actions for CIs. Azure pipelines and github actions are very similar and underneath they are based on same stack I believe... With github actions there is 1 layer less to interact. If you have a working azure pipeline CI then moving it to github actions should be trivial.

@askhade I have replaced azure pipelines with github actions in #7.

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.

4 participants