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

[tuning] Interface for Fin #3330

Draft
wants to merge 34 commits into
base: develop
Choose a base branch
from
Draft

[tuning] Interface for Fin #3330

wants to merge 34 commits into from

Conversation

averinevg
Copy link
Collaborator

This PR adds interface for Fin


protected:
ConvSolver(const miopen::solver::SolverBase* solver_base,
uint64_t solver_id,
Copy link
Contributor

@DrizztDoUrden DrizztDoUrden Oct 23, 2024

Choose a reason for hiding this comment

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

[Suggestion] I would recommend using solver::Id instead of numeric ids. It would be hassle free to create this from either number or string.

@junliume
Copy link
Collaborator

@averinevg please resolve the conflicts and a few hip tidy issue. Thanks.

template <>
const auto& GetSolversInfo<ConvSolverInfo>()
{
static const std::unordered_map<std::string, ConvSolverInfo> solver_info = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wow, what a list! Is this generated, or maintained by hand? There are likely to be many additions coming with MX data types. Seems like it wouldn't be too hard to import it from a file that would be easier for external tools to interact with (e.g. json, csv).

In either case, it would be nice to have placeholders for the missing ordinals. That way we can know at a glance if an algo was deleted or otherwise expected to be missing. Otherwise, it looks like something may have been missed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This list is filled manually. It contains all the solvers for the specified primitives currently available in the library. This is needed to check that the interface provides correct data. However, we don't need such a large list; a dozen solvers would be sufficient. There is also no need to update it when new solvers are added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants