Skip to content

Conversation

@hageboeck
Copy link
Member

This takes a few RooFit-related commits out of #18083.

  • The backend-related commit fixes the tests in case root has been configured with -Dclad=off.
  • The other two move over to explicitly registering/writing histograms instead of relying on implicit ownership. This way, the tests are independent of whether implicit registration is used or not.

Note: This might need #20888 to be merged first, in order not to issue warnings about histograms being replaced.

Although fallbacks to the cpu backend have been implemented, trying to
test RooFit without clad leads to a gtest failure, since the test suite
will be instantiated three times with the same backend.
Also add an assertion to stop the test when executed in the wrong
directory. Previously, the test would crash.
Copy link
Contributor

@guitargeek guitargeek left a comment

Choose a reason for hiding this comment

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

See inline comment


#define ROOFIT_EVAL_BACKENDS ROOFIT_EVAL_BACKEND_LEGACY ROOFIT_EVAL_BACKEND_CUDA RooFit::EvalBackend::Cpu()

#ifdef ROOFIT_CLAD
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be necessary because RooFit::EvalBackend::CodegenNoGrad() is available also without Clad, and ROOFIT_EVAL_BACKENDS is already defined to be empty if ROOFIT_CLAD is not defined. The problem must be somewhere else I think

Copy link
Member Author

Choose a reason for hiding this comment

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

The failure is:
In gtest-roofit-histfactory-testHistFactory

 [#0] ERROR:InputArguments -- RooFit was built without clad. Codegen backends are unavailable. Falling back to default.
314: 
314: [ FATAL ] /usr/include/gtest/internal/gtest-param-util.h:589:: Condition test_param_names.count(param_name) == 0 failed. Duplicate parameterized test name 'EquidistantBins_OverallSyst_Backend_cpu', in /home/stephan/code/root/roofit/histfactory/test/testHistFactory.cxx line 706

And in gtest-roofit-roofitcore-testRooMinimizer:

 [#0] ERROR:InputArguments -- RooFit was built without clad. Codegen backends are unavailable. Falling back to default.
387: 
387: [ FATAL ] /usr/include/gtest/internal/gtest-param-util.h:589:: Condition test_param_names.count(param_name) == 0 failed. Duplicate parameterized test name 'EvalBackendcpu', in /home/stephan/code/root/roofit/roofitcore/test/testRooMinimizer.cxx line 163

Running the preprocessor on the latter, I see that ROOFIT_EVAL_BACKENDS_WITH_CODEGEN expands to
RooFit::EvalBackend::Legacy(), RooFit::EvalBackend::Cpu(), RooFit::EvalBackend::CodegenNoGrad()

The return values of these functions are (when clad is off):
Legacy, Cpu, Cpu. The doubling of Cpu is why gtest aborts the test.

I fixed this by not adding CodegenNoGrad(), so the macro would expand to
Legacy, Cpu

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, then the problem is that CodegenNoGrad() unnecessarily falls back to Cpu. I'll see how I can fix this.

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants