-
Notifications
You must be signed in to change notification settings - Fork 86
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
add Schwarz config #1658
add Schwarz config #1658
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I would suggest to not instantiate type_descriptor
versions, which are not allowed.
core/config/type_descriptor.cpp
Outdated
template type_descriptor make_type_descriptor<std::complex<float>, int64>(); | ||
template type_descriptor make_type_descriptor<std::complex<double>, int64>(); | ||
// global_index: void | ||
template type_descriptor make_type_descriptor<void, void, void>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the documentation of type_descriptor
, void
is only allowed for the first template argument. So we shouldn't instantiate the other cases here. I think you can also use our normal macros then.
* @param index_typestr the index type string. "void" means no default. | ||
* @param global_index_typestr the global index type string. "void" means | ||
* no default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @param index_typestr the index type string. "void" means no default. | |
* @param global_index_typestr the global index type string. "void" means | |
* no default. | |
* @param index_typestr the index type string. | |
* @param global_index_typestr the global index type string. |
The doc above states that only value_type
may be void
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, the value in the value "void"
just mentions it is a possible value like float32, float64, int32 not it is for valuetype
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I misunderstood it. But I remember that we allow only the value_type
to be set in the config file, so it shouldn't be allowed to have no default for the index types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can only set up value type in file now. with void in index or global index type, user will get error when they use some solver requiring the index type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be useful only when user wants to ensure solver only uses the type provided by user.
It will be quite rare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed them for now
2f472f6
to
7d1ef72
Compare
7d1ef72
to
be85edb
Compare
Co-authored-by: Marcel Koch <marcel.koch@kit.edu> Co-authored-by: Pratik Nayak <pratikvn@protonmail.com>
be85edb
to
9335476
Compare
This PR adds the schwarz config.
Following the current decision, only allow the global index setting from the
type_descriptor
.Because sizeof(global_index_type) >= sizeof(index_type), there is an invalid configuration (int64, int32) such that we can not use current dispatch. We only have this case, so I directly write it into the schwarz not create another generalized version for dispatch.