[RF] Show INFO messages on the FastEvaluations topic by default - #23193
Open
guitargeek wants to merge 2 commits into
Open
[RF] Show INFO messages on the FastEvaluations topic by default#23193guitargeek wants to merge 2 commits into
guitargeek wants to merge 2 commits into
Conversation
Test Results 23 files 23 suites 3d 19h 7m 30s ⏱️ For more details on these failures, see this check. Results for commit 68743d2. ♻️ This comment has been updated with latest results. |
The FastEvaluations topic hosts two messages that advise implementing a faster evaluation interface for a class: one from the generic RooAbsReal::doEval() implementation, which falls back to scalar evaluate() calls in a loop, and one from RooFit::Evaluator, which reports nodes that had to be evaluated on the CPU because their class doesn't support CUDA. Both are emitted while the computation graph is evaluated, e.g. in every minimizer iteration, so with the FastEvaluations topic active a single fit would print them hundreds of times. Deduplicate both messages on the class name, such that each class is reported exactly once. For the GPU message, this replaces the per-node "hasLogged" guard, which only suppressed repetitions for a given node of a given Evaluator and therefore still repeated the same advice for every instance of a class in the computation graph, and again in every fit. The flag is kept as a cheap guard so that the hot evaluation path doesn't take the lookup more than once per node, and the message is reworded to refer to the class instead of the individual argument. As before, the messages are only emitted if the evaluated node is not a scalar, because there would be no speedup benefit otherwise. This is a prerequisite for making the FastEvaluations topic visible by default.
The FastEvaluations messages inform the user that a class does not implement the batch evaluation interface (doEval()), falling back to scalar evaluate() calls. Since the vectorizing CPU backend is now the default for likelihood evaluations, this information is relevant to regular users with custom classes, and it helps tracing down remaining RooFit classes without a doEval() implementation (see the corresponding item in issue root-project#6557). So far, the messages were effectively invisible: the topic was not part of any default stream, so nobody saw them unless they manually added a message stream. Add the topic to the default INFO stream and update the stale class documentation to match the actual default stream configuration.
guitargeek
force-pushed
the
fastevaluations-default-visible
branch
from
September 4, 2026 08:13
0ff19a9 to
68743d2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The FastEvaluations messages inform the user that a class does not implement the batch evaluation interface (doEval()), falling back to scalar evaluate() calls. Since the vectorizing CPU backend is now the default for likelihood evaluations, this information is relevant to regular users with custom classes, and it helps tracing down remaining RooFit classes without a doEval() implementation (see the corresponding item in issue #6557). So far, the messages were effectively invisible: the topic was not part of any default stream, so nobody saw them unless they manually added a message stream.
Add the topic to the default INFO stream and update the stale class documentation to match the actual default stream configuration.