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

[Math] Fix template Fitter::SetFCN and FitFCN #10737

Merged
merged 2 commits into from
Jun 10, 2022

Conversation

lmoneta
Copy link
Member

@lmoneta lmoneta commented Jun 10, 2022

This Pull request fixes the handling of the FCN function provided when using the template
signature of SetFCN and FitFCN.

This PR fixes #10732

This is a fix for the changes introduced in a previous commit (see PR root-project#10650)
that avoids the cloininc of FCN.
When the FCN is provided as a Funcotr class or a Lambda an internal wrapper object is created in the templated Fitter::SetFCN method that needs to be cloned in order to have it not deleted after exiting SetFCN.
The tutorial has been moved to use the Fitter class instead of the old legacy  TVirtualFitter.
The tutorial uses the template Fitter::SetFCN funciton.
@lmoneta lmoneta requested a review from couet as a code owner June 10, 2022 15:56
@phsft-bot
Copy link
Collaborator

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1015/python3, mac11/cxx17, windows10/cxx14
How to customize builds

@lmoneta lmoneta requested a review from guitargeek June 10, 2022 15:57
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.

Looks good to me! So if the user specifies the function it indeed needs to be cloned because it might go out of scope.

@phsft-bot
Copy link
Collaborator

Build failed on mac11/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@lmoneta
Copy link
Member Author

lmoneta commented Jun 10, 2022

The cloning happens only for the internal created WrappedMultiFunction not for the user provided object.

@lmoneta lmoneta merged commit 6f834eb into root-project:master Jun 10, 2022
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.

CMSSW build fails with root master 4c13caa0ac
3 participants