add file config example with the configs #1662

yhmtsai · 2024-08-06T09:31:15Z

This PR adds the example to show the solver configuration based on the file.
This PR aims to set up config close to the existing example without any registry usage.
Note. the stopping criterion is different among these configs.

cg.json: simple-solver
blockjacobi-cg.json: preconditioned-solver
ir.json: iterative-refinement
parilu.json: ilu-preconditioned-solver (by using factoization parameter directly)
pgm-multigrid-cg.json: multigrid-preconditioned-solver (set min_coarse_rows additionally due to this small example matrix)
mixed-pgm-multigrid-cg.json: mixed-multigrid-preconditioned-solver (assuming there are always more than one level)

There will be another one example to show the registry usage.

MarcelKoch

LGTM. I would prefer the json files merged.

examples/file-config-solver/config/blockjacobi-cg.json

examples/file-config-solver/build.sh

examples/file-config-solver/file-config-solver.cpp

thoasm

Right now, I only comment until the discussion is resolved.
I would like all configuration files to be tested and part of the CI.
Other than that, I only have nits.

examples/file-config-solver/doc/tooltip

examples/file-config-solver/file-config-solver.cpp

examples/file-config-solver/config/blockjacobi-cg.json

thoasm · 2024-08-28T13:36:02Z

examples/file-config-solver/file-config-solver.cpp

+    // mixed-pgm-multigrid-cg.json: mixed-multigrid-preconditioned-solver
+    //                              (assuming there are always more than one
+    //                              level)
+    auto config = gko::ext::config::parse_json_file(configfile);


My main concern is that this only tests a single file as part of our automatic tests. It doesn't check the other files, so we couldn't detect if we broke the compatibility.

I am not sure whether using examples as kind of integration test is a good idea.
I have add the test to check they can be runnable

thoasm · 2024-08-28T13:44:47Z

examples/CMakeLists.txt

@@ -20,6 +20,7 @@ set(EXAMPLES_EXEC_LIST
 set(EXAMPLES_LIST
    ${EXAMPLES_EXEC_LIST}
    custom-stopping-criterion
+    file-config-solver


Does this mean they are not run as part of our CI?
Is there a reason why we don't?

I have added test which only checks it can run properly.

I do not reuse the existing one because it needs to accept additional parameter

MarcelKoch

I still think that

JSON is not the right format
The files should be combined, and annotated with comments

But I won't hold up the PR.

MarcelKoch

you also need to add the file to doc/examples/examples.hpp.in

yhmtsai · 2024-09-10T18:56:06Z

I think json does not support the comment in the file.

My thought on this example is to show user can provide any kind of new config and then they can get a new solver without recompilation.
Combining them and accessing by key is somehow in the half-way. When they add a new solver to try, they need to recompile the program for the new key which is conflicted with the file config benefit.

Json is not suitable format

It is another topic. this example does not tend to handle this discussion.

greole

LGTM.

examples/file-config-solver/file-config-solver.cpp

examples/file-config-solver/doc/intro.dox

examples/file-config-solver/file-config-solver.cpp

Co-authored-by: Marcel Koch <marcel.koch@kit.edu>

Co-authored-by: Marcel Koch <marcel.koch@kit.edu> Co-authored-by: Thomas Grützmacher <thomas.gruetzmacher@kit.edu>

Co-authored-by: Marcel Koch <marcel.koch@kit.edu>

Co-authored-by: Gregor Olenik <gregor.olenik@web.de>

sonarcloud · 2024-10-23T12:30:59Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.8% Coverage on New Code
12.1% Duplication on New Code

See analysis details on SonarCloud

yhmtsai added the 1:ST:ready-for-review This PR is ready for review label Aug 6, 2024

yhmtsai requested review from a team August 6, 2024 09:31

yhmtsai self-assigned this Aug 6, 2024

ginkgo-bot added reg:build This is related to the build system. reg:example This is related to the examples. labels Aug 6, 2024

MarcelKoch requested changes Aug 14, 2024

View reviewed changes

MarcelKoch added this to the Ginkgo 1.9.0 milestone Aug 26, 2024

thoasm reviewed Aug 28, 2024

View reviewed changes

yhmtsai force-pushed the config_example branch from e25ce2f to e096a21 Compare September 9, 2024 20:25

yhmtsai requested review from MarcelKoch and thoasm September 9, 2024 20:28

MarcelKoch approved these changes Sep 10, 2024

View reviewed changes

MarcelKoch requested changes Sep 10, 2024

View reviewed changes

yhmtsai force-pushed the config_example branch from e096a21 to 7a6ee17 Compare September 10, 2024 18:47

MarcelKoch approved these changes Sep 11, 2024

View reviewed changes

yhmtsai force-pushed the config_example branch from 7a6ee17 to b2fa075 Compare October 17, 2024 14:16

greole self-requested a review October 18, 2024 09:11

greole approved these changes Oct 18, 2024

View reviewed changes

yhmtsai force-pushed the config_example branch from b2fa075 to ae05ba7 Compare October 18, 2024 11:00

yhmtsai added 1:ST:ready-to-merge This PR is ready to merge. and removed 1:ST:ready-for-review This PR is ready for review labels Oct 18, 2024

yhmtsai force-pushed the config_example branch 4 times, most recently from bae74ac to 8eb792f Compare October 20, 2024 22:41

yhmtsai added the 1:ST:skip-full-test label Oct 22, 2024

yhmtsai and others added 2 commits October 22, 2024 17:36

add file config example with the configs

42ac678

update build.sh to c++17 in example

5de2254

Co-authored-by: Marcel Koch <marcel.koch@kit.edu>

yhmtsai and others added 3 commits October 22, 2024 17:36

add test and update documentation

3051acb

Co-authored-by: Marcel Koch <marcel.koch@kit.edu> Co-authored-by: Thomas Grützmacher <thomas.gruetzmacher@kit.edu>

add missing example link

8fb6a5a

Co-authored-by: Marcel Koch <marcel.koch@kit.edu>

use milliseconds directly and update documentation

9f0469c

Co-authored-by: Gregor Olenik <gregor.olenik@web.de>

yhmtsai force-pushed the config_example branch 2 times, most recently from ee9aaac to 0d68ea2 Compare October 22, 2024 20:52

yhmtsai removed the 1:ST:skip-full-test label Oct 22, 2024

fix windows execution test by using std::filesystem

05742fe

yhmtsai force-pushed the config_example branch from 0d68ea2 to 05742fe Compare October 22, 2024 21:02

yhmtsai merged commit 272b4ff into develop Oct 23, 2024
8 of 11 checks passed

yhmtsai deleted the config_example branch October 23, 2024 08:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add file config example with the configs #1662

add file config example with the configs #1662

yhmtsai commented Aug 6, 2024

MarcelKoch left a comment

thoasm left a comment

thoasm Aug 28, 2024

yhmtsai Sep 9, 2024

thoasm Aug 28, 2024

yhmtsai Sep 9, 2024

yhmtsai Sep 9, 2024

MarcelKoch left a comment

MarcelKoch left a comment

yhmtsai commented Sep 10, 2024

greole left a comment

sonarcloud bot commented Oct 23, 2024

add file config example with the configs #1662

add file config example with the configs #1662

Conversation

yhmtsai commented Aug 6, 2024

MarcelKoch left a comment

Choose a reason for hiding this comment

thoasm left a comment

Choose a reason for hiding this comment

thoasm Aug 28, 2024

Choose a reason for hiding this comment

yhmtsai Sep 9, 2024

Choose a reason for hiding this comment

thoasm Aug 28, 2024

Choose a reason for hiding this comment

yhmtsai Sep 9, 2024

Choose a reason for hiding this comment

yhmtsai Sep 9, 2024

Choose a reason for hiding this comment

MarcelKoch left a comment

Choose a reason for hiding this comment

MarcelKoch left a comment

Choose a reason for hiding this comment

yhmtsai commented Sep 10, 2024

greole left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 23, 2024

Quality Gate passed