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

Check config key #1792

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Check config key #1792

wants to merge 6 commits into from

Conversation

yhmtsai
Copy link
Member

@yhmtsai yhmtsai commented Feb 18, 2025

This PR adds the check to verify the keys in config are allowed by predefined keys set.
typo in config is quite easy in the file because we do not have auto-complete for that
It avoids typo or misusage of parameters from user such that the configuring solver is not what users expect without notice.

Isome thoughts will be nice.
allowed_keys preparation: predefine it as early as possible or insert the key into the set close to the condition
I am predefining it as early as possible. but I think the second way might makes sense when the class has long list of keys like multigrid. It makes the manual type together and reduce the chance that missing deleting one of condition and allowed key. The first one is somehow easy to set with constructor not many lines of set.insert("key")

@yhmtsai yhmtsai added the 1:ST:need-feedback The PR is somewhat ready but feedback on a blocking topic is required before a proper review. label Feb 18, 2025
@yhmtsai yhmtsai requested a review from a team February 18, 2025 16:01
@yhmtsai yhmtsai self-assigned this Feb 18, 2025
@ginkgo-bot ginkgo-bot added reg:testing This is related to testing. mod:core This is related to the core module. type:solver This is related to the solvers labels Feb 18, 2025
@yhmtsai yhmtsai added 1:ST:ready-for-review This PR is ready for review and removed 1:ST:need-feedback The PR is somewhat ready but feedback on a blocking topic is required before a proper review. labels Feb 20, 2025
@ginkgo-bot
Copy link
Member

Error: The following files need to be formatted:

common/cuda_hip/matrix/ell_kernels.cpp
common/cuda_hip/matrix/sellp_kernels.cpp
common/cuda_hip/matrix/sparsity_csr_kernels.cpp
common/unified/matrix/dense_kernels.template.cpp
core/matrix/csr.cpp
core/test/matrix/csr.cpp
dpcpp/matrix/csr_kernels.dp.cpp
dpcpp/matrix/ell_kernels.dp.cpp
dpcpp/matrix/sellp_kernels.dp.cpp
dpcpp/matrix/sparsity_csr_kernels.dp.cpp
include/ginkgo/core/matrix/csr.hpp
omp/matrix/csr_kernels.cpp
omp/matrix/dense_kernels.cpp
omp/matrix/ell_kernels.cpp
omp/matrix/fbcsr_kernels.cpp
omp/matrix/sellp_kernels.cpp
omp/matrix/sparsity_csr_kernels.cpp
reference/matrix/csr_kernels.cpp
reference/matrix/dense_kernels.cpp
reference/matrix/ell_kernels.cpp
reference/matrix/fbcsr_kernels.cpp
reference/matrix/sellp_kernels.cpp
reference/matrix/sparsity_csr_kernels.cpp
reference/test/matrix/csr_kernels.cpp
reference/test/matrix/dense_kernels.cpp
reference/test/matrix/ell_kernels.cpp
reference/test/matrix/sellp_kernels.cpp
reference/test/matrix/sparsity_csr_kernels.cpp
test/factorization/ic_kernels.cpp
test/factorization/ilu_kernels.cpp
test/matrix/csr_kernels2.cpp
test/matrix/matrix.cpp

You can find a formatting patch under Artifacts here or run format! if you have write access to Ginkgo

@yhmtsai yhmtsai requested a review from fritzgoebel February 20, 2025 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1:ST:ready-for-review This PR is ready for review mod:core This is related to the core module. reg:testing This is related to testing. type:solver This is related to the solvers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants