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

[tuner] Generalizes Configuration dataclass #345

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mihaescuvlad
Copy link
Contributor

@mihaescuvlad mihaescuvlad commented Oct 27, 2024

Notes:

  • Splits Configuration into LLVMGPUConfiguration & LLVMCPUConfiguration
  • Adds device_target to the MlirRegex dataclass
  • Adds SolutionGenerationStrategy for decoupling the solution generation from the configuration
  • Adds SolutionStrategyFactory to fetch the proper solution strategy based on target architecture
  • Adds Mmt4d pipeline support

Tests:

  • Architecture tests:
    • Adds separate tests for GPU and CPU solution & constraints generation
  • Tuning tests:
    • Adds separate tests for GPU and CPU tuning
    • Adds tests for Mmt4dTuner

Improved Usage:

  • Automatic Architecture Selection: The target architecture is automatically detected from the input.mlir file. The correct configuration and solution strategy are chosen accordingly, with no user input required, apart from compiling the input.mlir for the intended architecture in advance

Copy link
Member

@kuhar kuhar left a comment

Choose a reason for hiding this comment

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

I think the key consideration should be how to generate the constraints. I think this should belong to each configuration: given a problem size and dispatch kind, it should know how to generate constraints and perform the lookup in the model generated by Z3 to apply the assignment

tuner/tuner/candidate_gen.py Outdated Show resolved Hide resolved
Comment on lines 292 to 293
@dataclass
class BaseConfigurationBuilder:
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need these builders?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was one of the only ways of creating the configurations that I could think of while trying to avoid hacks. I tried using a strategy pattern before, but it was slightly less clear than using builders + factory. It's still not a perfect approach though, because the generate_solutions method still loses some of the clarity. I'm considering moving that method inside of the configuration dataclasses and getting rid of the builder while keeping a factory for creating the configurations

@mihaescuvlad mihaescuvlad force-pushed the users/mihaescuvlad/configuration-generalization branch from e815981 to 8f69c0c Compare October 31, 2024 16:29
@mihaescuvlad mihaescuvlad force-pushed the users/mihaescuvlad/configuration-generalization branch from 8f69c0c to 28d3a06 Compare November 1, 2024 11:12
@mihaescuvlad mihaescuvlad marked this pull request as ready for review November 5, 2024 08:56
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.

2 participants