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

ParameterHandler::add_parameter(): do not call action #14562

Merged
merged 1 commit into from
Mar 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions doc/news/changes/incompatibilities/20221211Munch
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Fixed: The function ParamerHandler::add_parameter() used to
call the internal action. Within that step, the action
converts the default value to a string and back afterwards.
This can lead to round-off errors so that the default
values might change in the case of floating-point numbers.
The action is not called any more during ParamerHandler::add_parameter(),
fixing the problem.
<br>
(Peter Munch, Magdalena Schreter, 2022/12/11)
17 changes: 11 additions & 6 deletions include/deal.II/base/parameter_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -1137,10 +1137,14 @@ class ParameterHandler : public Subscriptor
*
* The action is executed in three different circumstances:
* - With the default value of the parameter with name @p name, at
* the end of the current function. This is useful because it allows
* for the action to execute whatever it needs to do at least once
* for each parameter, even those that are not actually specified in
* the input file (and thus remain at their default values).
* the end of the current function if @p execute_action is set to
* true. This is useful because it allows for the action to execute
* whatever it needs to do at least once for each parameter, even
* those that are not actually specified in the input file (and
* thus remain at their default values). Note that if the action
* is executed, it converts the default value to a string and back
* afterwards. This can lead to round-off errors so that the default
* values might change in the case of floating-point numbers.
* - Within the ParameterHandler::set() functions that explicitly
* set a value for a parameter.
* - Within the parse_input() function and similar functions such
Expand Down Expand Up @@ -1173,7 +1177,8 @@ class ParameterHandler : public Subscriptor
*/
void
add_action(const std::string & entry,
const std::function<void(const std::string &value)> &action);
const std::function<void(const std::string &value)> &action,
const bool execute_action = true);
Comment on lines 1178 to +1181
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have to adjust the documentation of the function accordingly. The first bullet point of the documentation states that the action is executed at the end of the call to this function, but this is now no longer generally true.


/**
* Declare a new entry name @p entry, set its default value to the content of
Expand Down Expand Up @@ -2353,7 +2358,7 @@ ParameterHandler::add_parameter(const std::string & entry,
parameter = Patterns::Tools::Convert<ParameterType>::to_value(
val, *patterns[pattern_index]);
};
add_action(entry, action);
add_action(entry, action, false);
}

DEAL_II_NAMESPACE_CLOSE
Expand Down
14 changes: 8 additions & 6 deletions source/base/parameter_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,8 @@ ParameterHandler::declare_entry(const std::string & entry,
void
ParameterHandler::add_action(
const std::string & entry,
const std::function<void(const std::string &)> &action)
const std::function<void(const std::string &)> &action,
const bool execute_action)
{
actions.push_back(action);

Expand All @@ -898,11 +899,12 @@ ParameterHandler::add_action(
entries->put(get_current_full_path(entry) + path_separator + "actions",
Utilities::int_to_string(actions.size() - 1));


// as documented, run the action on the default value at the very end
const std::string default_value = entries->get<std::string>(
get_current_full_path(entry) + path_separator + "default_value");
action(default_value);
if (execute_action)
{
const std::string default_value = entries->get<std::string>(
get_current_full_path(entry) + path_separator + "default_value");
action(default_value);
}
}


Expand Down
42 changes: 42 additions & 0 deletions tests/parameter_handler/add_parameter_01.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// ---------------------------------------------------------------------
//
// Copyright (C) 2022 by the deal.II authors
//
// This file is part of the deal.II library.
//
// The deal.II library is free software; you can use it, redistribute
// it, and/or modify it under the terms of the GNU Lesser General
// Public License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
// The full text of the license can be found in the file LICENSE.md at
// the top level directory of deal.II.
//
// ---------------------------------------------------------------------



// check that ParameterHandler::add_parameter() does not modify the
// default value

#include <deal.II/base/parameter_handler.h>

#include "../tests.h"

using namespace dealii;

int
main()
{
initlog();

double a = std::numeric_limits<double>::lowest();

AssertThrow(a == std::numeric_limits<double>::lowest(), ExcInternalError());

ParameterHandler prm;
prm.add_parameter("test", a);

AssertThrow(a == std::numeric_limits<double>::lowest(), ExcInternalError());

deallog << "OK!" << std::endl;
}
2 changes: 2 additions & 0 deletions tests/parameter_handler/add_parameter_01.output
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

DEAL::OK!