ParameterHandler::add_parameter(): do not call action #14562
Conversation
peterrum
force-pushed
the
add_parameter_no_action
branch
from
dce2c20
to
97551e8
Compare
| call the internal action. Since the action converts during | ||
| that step the default value to a string and afterwards back, | ||
| this could lead to rounding-off errors so that the default | ||
| values might change in the case of floating-point numbers. |
There was a problem hiding this comment.
| call the internal action. Since the action converts during | |
| that step the default value to a string and afterwards back, | |
| this could lead to rounding-off errors so that the default | |
| values might change in the case of floating-point numbers. | |
| 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. |
source/base/parameter_handler.cc
Outdated
| const std::string default_value = entries->get<std::string>( | ||
| get_current_full_path(entry) + path_separator + "default_value"); | ||
| action(default_value); | ||
| if (excecute_action) |
There was a problem hiding this comment.
| if (excecute_action) | |
| if (execute_action) |
peterrum
force-pushed
the
add_parameter_no_action
branch
from
97551e8
to
436ef17
Compare
|
@masterleinad Thanks for the review! |
| 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); |
There was a problem hiding this comment.
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.
source/base/parameter_handler.cc
Outdated
| action(default_value); | ||
| if (execute_action) | ||
| { | ||
| // as documented, run the action on the default value at the very end |
There was a problem hiding this comment.
Just remove this line -- it is no longer actually true.
|
Ping? |
mschreter
force-pushed
the
add_parameter_no_action
branch
from
436ef17
to
87d225c
Compare
mschreter
force-pushed
the
add_parameter_no_action
branch
from
87d225c
to
bbb1a22
Compare
We (@mschreter) made the changes! Thanks for the review! |
|
@bangerth Ping :-)? |
|
My apologies for forgetting about this :-( The logs are no longer available for the CI checks (they get deleted after some time), but I'm pretty sure that the patch is safe and so I will merge. Let's hope that I'm not going to make a fool out of myself for it... |
closes #13970
FYI @mschreter