Minimal stopping criteria config #1613

MarcelKoch · 2024-05-17T15:01:14Z

This PR adds a minimal specification for configuring stopping criteria. The test now contains an example for this configuration. Since the main implementation to configure the criteria is already implemented, this adds only a bit of dispatching.

This is intended to continue the discussion in #1392, while not blocking the release.

Todo:

discuss the specification
test all allowed key-value pairs

The stopping criteria can now be defined as:

criteria:
  value_type: float32 # optional 
  # only one of the following is required 
  relative_residual_norm: 1e-6
  initial_residual_norm: 1e-6
  absolute_residual_norm: 1e-6
  relative_implicit_residual_norm: 1e-6
  initial_implicit_residual_norm: 1e-6
  absolute_implicit_residual_norm: 1e-6
  time: 1000

yhmtsai · 2024-05-17T21:43:13Z

core/config/config_helper.cpp

+    }
+
+    if (config.get_tag() == pnode::tag_t::map) {
+        return parse_minimal_criteria(config, context, td);


Suggested change

return parse_minimal_criteria(config, context, td);

auto updated = config::update_type(td);

return parse_minimal_criteria(config, context, updated);

It is to support no valuetype available outside

I'm not sure what you mean here. I can specify the value type of the residual nom criterion, as you can see in the tests.

It's based on the type_descriptor from the outer loop.
I mean something like

"stop": { "value_type": "float64", "residual_norm": ... }

in case no precision information from outside or want to specify certain precision for stop.

following the above comment,

"stop": [ { "type": "ResidualNorm", "value_type": "float32", ... } ]

in multigrid or some solver without precision information.
it can be mapped to

"stop": { "value_type": "float32", "residual_norm": ... }

Or, we can say this rare case should use the map version not minimal version

I will try a bit to make it work. If it's a reasonable effort I will adapt it.

It was pretty simple to add, so it's now available.

yhmtsai · 2024-05-17T21:44:28Z

core/config/config_helper.cpp

+parse_or_get_criteria(const pnode& config, const registry& context,
+                      const type_descriptor& td)
+{
+    if (config.get_tag() == pnode::tag_t::array) {


The array can not accept 1 object now. you can use type to distinguish the path and reuse the parse_or_get_factory

core/config/config_helper.hpp

yhmtsai · 2025-02-13T13:36:59Z

core/config/config_helper.cpp

+    }
+
+    if (config.get_tag() == pnode::tag_t::map) {
+        return parse_minimal_criteria(config, context, td);


following the above comment,

"stop": [ { "type": "ResidualNorm", "value_type": "float32", ... } ]

in multigrid or some solver without precision information.
it can be mapped to

"stop": { "value_type": "float32", "residual_norm": ... }

yhmtsai · 2025-02-13T13:37:44Z

core/config/config_helper.cpp

+    }
+
+    if (config.get_tag() == pnode::tag_t::map) {
+        return parse_minimal_criteria(config, context, td);


Or, we can say this rare case should use the map version not minimal version

yhmtsai

LGTM! only nit left is that the updated_td can reuse the existing function

yhmtsai · 2025-02-13T15:17:53Z

core/config/config_helper.cpp

+    auto value_type_it = config.get_map().find("value_type");
+    type_descriptor updated_td =
+        value_type_it == config.get_map().end()
+            ? td
+            : type_descriptor(value_type_it->second.get_string(),
+                              td.get_local_index_typestr(),
+                              td.get_global_index_typestr());


you can also use updated_td = config::update_type(td);

Thanks for the hint!

sonarqubecloud · 2025-02-18T04:10:52Z

Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

codecov · 2025-02-18T04:16:03Z

Codecov Report

Attention: Patch coverage is 98.16514% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.48%. Comparing base (efbf245) to head (d89b027).
Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
core/config/config_helper.cpp	94.87%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1613      +/-   ##
===========================================
+ Coverage    88.77%   89.48%   +0.70%     
===========================================
  Files          808      808              
  Lines        66686    66723      +37     
===========================================
+ Hits         59201    59706     +505     
+ Misses        7485     7017     -468

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

upsj

LGTM!

Signed-off-by: Marcel Koch <marcel.koch@kit.edu>

- allow changing the value_type - fix docs - call `parse_or_get_criteria` directly from solver config Co-authored-by: Yu-Hsiang M. Tsai <yhmtsai@gmail.com>

ginkgo-bot · 2025-02-18T09:13:37Z

Error: PR already merged!

MarcelKoch self-assigned this May 17, 2024

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 May 17, 2024

MarcelKoch mentioned this pull request May 17, 2024

Factory config #1392

Merged

yhmtsai reviewed May 17, 2024

View reviewed changes

yhmtsai force-pushed the factory_config branch 2 times, most recently from 7735cc5 to 2629167 Compare May 27, 2024 09:22

Base automatically changed from factory_config to develop May 27, 2024 14:51

MarcelKoch force-pushed the minimal-stop-config branch from 0126f7d to 829b991 Compare May 29, 2024 07:49

MarcelKoch added the 1:ST:WIP This PR is a work in progress. Not ready for review. label Aug 15, 2024

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

MarcelKoch modified the milestones: Ginkgo 1.9.0, Ginkgo 1.10.0 Dec 9, 2024

MarcelKoch marked this pull request as ready for review February 13, 2025 11:56

fritzgoebel self-requested a review February 13, 2025 11:57

yhmtsai requested changes Feb 13, 2025

View reviewed changes

MarcelKoch force-pushed the minimal-stop-config branch from 829b991 to c1ce897 Compare February 13, 2025 15:06

MarcelKoch requested a review from yhmtsai February 13, 2025 15:08

MarcelKoch added 1:ST:ready-for-review This PR is ready for review and removed 1:ST:WIP This PR is a work in progress. Not ready for review. labels Feb 13, 2025

yhmtsai approved these changes Feb 13, 2025

View reviewed changes

MarcelKoch force-pushed the minimal-stop-config branch from c1ce897 to cd8fce8 Compare February 13, 2025 15:22

MarcelKoch 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 Feb 17, 2025

MarcelKoch force-pushed the minimal-stop-config branch from cd8fce8 to d89b027 Compare February 17, 2025 08:29

MarcelKoch requested a review from yhmtsai February 18, 2025 08:22

upsj approved these changes Feb 18, 2025

View reviewed changes

MarcelKoch force-pushed the minimal-stop-config branch from 3478d9a to aaaca49 Compare February 18, 2025 08:42

MarcelKoch and others added 5 commits February 18, 2025 09:58

[config] adds minimal stopping criteria specification

292dbbe

Signed-off-by: Marcel Koch <marcel.koch@kit.edu>

[config] adds test for minimal stopping criteria config

13fd50f

Signed-off-by: Marcel Koch <marcel.koch@kit.edu>

review updates:

b54469a

- allow changing the value_type - fix docs - call `parse_or_get_criteria` directly from solver config Co-authored-by: Yu-Hsiang M. Tsai <yhmtsai@gmail.com>

[config] add doc on minimal criteria

9663428

[config] more consistent examples

7f91879

MarcelKoch force-pushed the minimal-stop-config branch from aaaca49 to 7f91879 Compare February 18, 2025 08:58

MarcelKoch merged commit dcd910f into develop Feb 18, 2025
7 of 11 checks passed

MarcelKoch deleted the minimal-stop-config branch February 18, 2025 09:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minimal stopping criteria config #1613

Minimal stopping criteria config #1613

MarcelKoch commented May 17, 2024 •

edited

Loading

yhmtsai May 17, 2024

MarcelKoch May 21, 2024

yhmtsai May 22, 2024

yhmtsai Feb 13, 2025

yhmtsai Feb 13, 2025

MarcelKoch Feb 13, 2025

MarcelKoch Feb 13, 2025

yhmtsai May 17, 2024

yhmtsai Feb 13, 2025

yhmtsai Feb 13, 2025

yhmtsai left a comment

yhmtsai Feb 13, 2025

MarcelKoch Feb 13, 2025

sonarqubecloud bot commented Feb 18, 2025

codecov bot commented Feb 18, 2025

upsj left a comment

ginkgo-bot commented Feb 18, 2025

	return parse_minimal_criteria(config, context, td);
	auto updated = config::update_type(td);
	return parse_minimal_criteria(config, context, updated);

Minimal stopping criteria config #1613

Minimal stopping criteria config #1613

Conversation

MarcelKoch commented May 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yhmtsai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Feb 18, 2025

Quality Gate passed

codecov bot commented Feb 18, 2025

Codecov Report

upsj left a comment

Choose a reason for hiding this comment

ginkgo-bot commented Feb 18, 2025

MarcelKoch commented May 17, 2024 •

edited

Loading