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

Placing parameter structs outside their respective classes #199

Open
AmroAlJundi opened this issue Oct 9, 2022 · 0 comments
Open

Placing parameter structs outside their respective classes #199

AmroAlJundi opened this issue Oct 9, 2022 · 0 comments
Labels
state: inactive Issue is abandoned, but might become relavent at some point type: fix Iterations on existing features or infrastructure. Optimizations, refactoring, etc.

Comments

@AmroAlJundi
Copy link
Contributor

If I want to use MetisReordering with the ReorderBase function, I'd need to declare the parameters object alone and use templates for it:

MetisReorder<ull, ull, val>::MetisReorderParams params;
ull *perm = ReorderBase::Reorder<MetisReorder>(params, A, {&cpu_context}, true);

As you can see we need the template types for the params object. It isn't very aesthetically pleasing. The way to avoid this is to have users pass all the members of the params object to the Reorder call which is too many:

  struct MetisReorderParams : PreprocessParams {
    int64_t ctype = metis::METIS_CTYPE_RM;
    int64_t rtype = metis::METIS_RTYPE_SEP2SIDED;
    int64_t nseps = 1;
    int64_t numbering = 0;
    int64_t niter = 10;
    int64_t seed = 42;
    int64_t no2hop = 0;
    int64_t compress = 0;
    int64_t ccorder = 0;
    int64_t pfactor = 0;
    int64_t ufactor = 30;
  };

We can also declare this struct outside the MetisReorder class. But this solution isn't "general" since there are preprocessings in which the parameters are templated (in Permute, the ordering is dependent on IDType).

I'm leaning towards declaring the params structs outside their classes. I have three reasons:

  1. If the struct is not template-dependent, its declaration would be easy
  2. If it is templated, then it will be the same thing as if it were inside the class as far as the user is concerned:
    SomeReorderParams<int, int, int> params;
    // vs.
    SomeReorder<int,int,int>::SomeReorderParams params;
  3. Generally, if the struct is templated dependent, it is going to be created inside the class constructor itself.

The only problem is that when it was inside the class the namespacing allows the IDE to figure it out. But if the params struct and the class itself have the same prefix then it wouldn't be that bad; we can maintain the standard of ParamsTypeName = PreprocessingName+"Params"

@AmroAlJundi AmroAlJundi added state: inactive Issue is abandoned, but might become relavent at some point type: fix Iterations on existing features or infrastructure. Optimizations, refactoring, etc. labels Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state: inactive Issue is abandoned, but might become relavent at some point type: fix Iterations on existing features or infrastructure. Optimizations, refactoring, etc.
Projects
None yet
Development

No branches or pull requests

1 participant