Skip to content

Conversation

amroakmal
Copy link
Collaborator

@amroakmal amroakmal commented Aug 4, 2025

Description

Currently checks such as self.spec.satisfies("affinity=none"): will not error if there is a spelling/syntax mistake within the argument, e.g. self.spec.satisfies("affinity=non"): (missing "e"). This will of course not be a syntax error, and the expected usage from the command line will not satisfy this condition. The changes in this PR uses Enums to avoid these programming mistakes, which are potentially difficult to debug.

Adding/modifying core functionality, CI, or documentation:

  • Refactor lib/benchpark/experiment.py

@amroakmal amroakmal added the WIP A work-in-progress not yet ready to commit label Aug 4, 2025
@github-actions github-actions bot added the feature New feature or request label Aug 4, 2025
Copy link
Collaborator

@michaelmckinsey1 michaelmckinsey1 left a comment

Choose a reason for hiding this comment

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

From @scheibelp :

  1. Import respective enums from class (cuda enum should come from cuda.py)
  2. make variable names more succinct where possible

return "single_node" if self.spec.satisfies("+single_node") else ""


class AffinityVariantValues(str, Enum):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this could be AffinityEnum and we could make a base enum class for the tuple_values methods etc. that are shared across enums

@michaelmckinsey1 michaelmckinsey1 changed the title [WIP] Trial 1 [WIP] Use Enums to Enforce Syntax Aug 4, 2025
@amroakmal amroakmal requested a review from scheibelp August 4, 2025 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request WIP A work-in-progress not yet ready to commit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants