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

Make is_cpu_op default to False in ETFeeder #180

Merged
merged 3 commits into from
Jan 30, 2025

Conversation

willjwon
Copy link
Contributor

Summary

At least in ASTRA-sim, only Chakra nodes with is_cpu_op set to False will be simulated. Given the objective of the Chakra (capturing distributed ML workloads), it seems it makes more sense to set is_cpu_op False by default when reading a Chakra node in ETFeeder: when it is not explicitly set to True.

@willjwon willjwon requested a review from a team as a code owner January 29, 2025 14:33
Copy link

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

@JoongunPark
Copy link
Contributor

This PR makes sense to me and also solves the problem mentioned in this PR (#159)

@spandoescode
Copy link

Looks good to me! Fixes downstream issues in Astra-sim

Copy link
Contributor

@tushar-krishna tushar-krishna left a comment

Choose a reason for hiding this comment

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

This looks good! This is just the default value update. Helps some downstream tools that don't have checks for is_cpu_op. In real traces this field will be set.

@tushar-krishna tushar-krishna merged commit aa42ea5 into mlcommons:main Jan 30, 2025
9 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jan 30, 2025
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.

5 participants