Skip to content

fixes is_cpu_op conversion in constructor#60

Merged
srinivas212 merged 1 commit intomlcommons:mainfrom
AlexandruAntonescuKeysight:is_cpu_op_fix
May 15, 2024
Merged

fixes is_cpu_op conversion in constructor#60
srinivas212 merged 1 commit intomlcommons:mainfrom
AlexandruAntonescuKeysight:is_cpu_op_fix

Conversation

@AlexandruAntonescuKeysight
Copy link
Copy Markdown
Contributor

Summary

This PR fixes the is_cpu_op() method which was always returning false, due to a wrong conversion in the constructor.

  • the is_cpu_op() method is returning bool, but variable is_cpu_op_ was of type uint32_t --> changed it to bool
  • the conversion in the constructor of ETFeederNode was being done in uint32_t instead of bool --> changed it to bool
  • in trace, the value for is_cpu_op attribute is a bool

Test Plan

I've tested using 2 different traces, converted them to json using chakra_jsonizer and checked if the value returned by is_cpu_op() method for a specific node is the same with the one from the json.

@AlexandruAntonescuKeysight AlexandruAntonescuKeysight requested a review from a team as a code owner May 14, 2024 15:02
@github-actions
Copy link
Copy Markdown

github-actions bot commented May 14, 2024

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

@srinivas212
Copy link
Copy Markdown
Contributor

@AlexandruAntonescuKeysight merging this PR. We should discuss adding unit tests for C++ code.

@srinivas212 srinivas212 merged commit 0a20bf8 into mlcommons:main May 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 15, 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.

2 participants