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

preparing sync between branches to avoid divergence #330

Merged
merged 45 commits into from
Oct 4, 2024
Merged

Conversation

gfursin
Copy link
Contributor

@gfursin gfursin commented Oct 3, 2024

As discussed (#327 , #257), let's start syncing branches to avoid further divergence and have time to test current workflows from main & mlperf-inference before changing json to yaml ...

@anandhu-eng and @arjunsuresh - can you please check how it influences the mlperf-inference branch, please?

BTW, I somehow thought that we synced all branches about a month ago so I am not sure why there is such a difference ...

The code should be mostly stable in all branches so it should be possible to check and test changes soon.

gfursin and others added 30 commits September 20, 2024 15:27
 * added more standard Nvidia Docker configuration for PyTorch
Update customize.py | Fix download-file on windows when downloaded fi…
various updates for CM tools for Windows and better debugging
turn on tests on Windows
@gfursin gfursin requested a review from a team as a code owner October 3, 2024 15:24
Copy link

github-actions bot commented Oct 3, 2024

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

## CM concepts

* https://doi.org/10.5281/zenodo.8105339
* https://arxiv.org/abs/2406.16791
Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove this arxiv paper - it has many factually wrong things. You had earlier agreed to fix them.

Copy link
Contributor Author

@gfursin gfursin Oct 4, 2024

Choose a reason for hiding this comment

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

?
It has a few minor typos that I still need to clean up but it was not urgent. But if you feel that something is wrong, I will remove it for now to discuss it later. There are also many other things I would like to clarify with the very recent CM/CM4MLOps developments while I was on a sick leave before updating the paper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the PR: #336

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you Grigori. Yes, not just the typos - "Arjun Suresh - a senior software engineer from cKnowledge.org" - I was at OctoML and only later at cKnowledge and even there the title was not the same. And I think in the last section "Future Plans" you made a typo to use "We" as everywhere else in the PDF it is "I".

@@ -1,5 +1,5 @@
Copyright (c) 2021-2024 MLCommons

The cTuning foundation and OctoML donated this project to MLCommons to benefit everyone.
Grigori Fursin, the cTuning foundation and OctoML donated this project to MLCommons to benefit everyone.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should agree on the history and keep it stable - it doesn't look good if it keeps on changing. Technically cm4mlops and all the code inside it started within MLCommons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parts of the code comes from older versions of CK and CM and from the https://github.com/mlcommons/ck where it was originally resided. We have to reflect that to avoid legal problems in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally started developing CM4MLOps as an integral part of CM while reusing some parts of CK and CK4MLOps that I developed at cTuning so it has to have a common history and license.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

The OctoAI blog has nothing to do with CM as it is talking about results taken using CK. Anyway I do not want to argue over these things. Since none of the projects are new, I request you to finalize the historical attributions and contributions and and keep them constant. This can help others decide in using the project. If anyone has a concern that the history of a project can change in future no one will be happy to contribute to it. Or else, let git history take care of them automatically as done in most other MLCommons repositories.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Agree! Thank you!

But we still have a problem with the conflict in .github/workflows/test-mlperf-inference-resnet50.yml .

May I ask you to help resolve it and merge the PR, please?

Thanks a lot for your help - very appreciated!

@gfursin
Copy link
Contributor Author

gfursin commented Oct 4, 2024

@anandhu-eng @arjunsuresh - I don't understand why but the button to "Resolve conflicts" is gray and I have a problem resolving a small conflict in 1 file before merging this PR...

Do you think you can help me resolve the conflict in .github/workflows/test-mlperf-inference-resnet50.yml, please?

Thank you very much in advance!

@gfursin gfursin merged commit 06da009 into mlperf-inference Oct 4, 2024
34 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants