[clad] Initial support of hessian calculation in TFormula using clad by sudo-panda · Pull Request #8371 · root-project/root

sudo-panda · 2021-06-08T12:51:00Z

TFormula already supports gradient calculation using clad. This commit extends that support for hessian calculation. Thus the hessian generation is done by clad and TFormula wraps it in a trampoline function.

phsft-bot · 2021-06-08T12:51:03Z

Can one of the admins verify this patch?

vgvassilev · 2021-06-08T14:37:23Z

@phsft-bot build!

cc: @ioanaif

phsft-bot · 2021-06-08T14:37:35Z

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

hist/hist/inc/TFormula.h

hist/hist/src/TFormula.cxx

vgvassilev · 2021-08-12T11:14:22Z

@phsft-bot build!

phsft-bot · 2021-08-12T11:14:28Z

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

phsft-bot · 2021-08-12T12:04:02Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-12T11:16:38.618Z] stderr: error: could not read '.git/rebase-apply/head-name': No such file or directory

vgvassilev · 2021-08-16T04:41:01Z

@phsft-bot build!

phsft-bot · 2021-08-16T04:41:08Z

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

phsft-bot · 2021-08-16T04:43:15Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-16T04:43:10.625Z] stderr: error: Failed to merge in the changes.
[2021-08-16T04:43:14.371Z] CMake Error at /home/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1045 (message):

phsft-bot · 2021-08-16T04:43:30Z

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-16T04:43:22.799Z] stderr: error: Failed to merge in the changes.
[2021-08-16T04:43:29.656Z] CMake Error at /mnt/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1045 (message):

phsft-bot · 2021-08-16T04:43:32Z

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

Errors:

[2021-08-16T04:43:31.462Z] CMake Error at /Users/sftnight/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1045 (message):

phsft-bot · 2021-08-16T04:43:44Z

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-16T04:43:43.383Z] CMake Error at /data/sftnight/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1045 (message):

phsft-bot · 2021-08-16T04:45:03Z

Build failed on mac1014/python3.
Running on macitois21.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-16T04:44:54.793Z] stderr: error: Failed to merge in the changes.
[2021-08-16T04:45:02.371Z] CMake Error at /Volumes/HD2/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1045 (message):

phsft-bot · 2021-08-16T04:51:34Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2021-08-16T04:51:33.592Z] CMake Error at C:/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1045 (message):

phsft-bot · 2021-08-16T14:23:09Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

sudo-panda · 2021-08-17T10:44:31Z

@lmoneta could you please review the PR?

vgvassilev · 2021-08-17T11:43:48Z

@phsft-bot build!

phsft-bot · 2021-08-17T11:43:54Z

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

phsft-bot · 2021-08-17T13:41:36Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-17T12:58:12.949Z] stderr: error: could not read '.git/rebase-apply/head-name': No such file or directory

hist/hist/inc/TFormula.h

hist/hist/src/TFormula.cxx

vgvassilev · 2021-08-18T08:56:43Z

@phsft-bot build!

phsft-bot · 2021-08-18T08:56:48Z

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

phsft-bot · 2021-08-18T08:58:52Z

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-4.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-18T08:58:50.669Z] CMake Error at /usr/local/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:165 (message):
[2021-08-18T08:58:51.184Z] CMake Error at /mnt/build/workspace/root-pullrequests-build/rootspi/jenkins/root-build.cmake:1117 (message):

phsft-bot · 2021-08-18T10:34:18Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-18T09:55:52.972Z] stderr: error: could not read '.git/rebase-apply/head-name': No such file or directory

vgvassilev · 2021-08-18T11:08:35Z

@phsft-bot build!

phsft-bot · 2021-08-18T11:08:40Z

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

hist/hist/inc/TFormula.h

hist/hist/src/TFormula.cxx

phsft-bot · 2021-08-18T12:18:46Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Errors:

[2021-08-18T11:40:59.097Z] stderr: error: could not read '.git/rebase-apply/head-name': No such file or directory

lmoneta

Very nice contribution! Thank you for this nice work.
It Looks good, I have just few comments.

hist/hist/inc/TFormula.h

hist/hist/test/TFormulaHessianTests.cxx

hist/hist/inc/TFormula.h

phsft-bot · 2021-08-18T15:42:13Z

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Errors:

[2021-08-18T15:41:54.965Z] MSBUILD : error MSB4166: Child node "3" exited prematurely. Shutting down. Diagnostic information may be found in files in "C:\Users\sftnight\AppData\Local\Temp" and will be named MSBuild_*.failure.txt. This location can be changed by setting the MSBUILDDEBUGPATH environment variable to a different directory.

sudo-panda · 2021-08-19T13:27:12Z

@lmoneta could you please look at the replies to your previous comments.

lmoneta

LGTM now.
I have just a small comment on the documentation

hist/hist/inc/TFormula.h

TFormula already supports gradient calculation using clad. This commit extends that support for hessian calculation. Thus the hessian generation is done by clad and TFormula wraps it in a trampoline function.

vgvassilev · 2021-08-20T08:35:05Z

@phsft-bot build!

phsft-bot · 2021-08-20T08:35:10Z

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

oshadura · 2021-08-23T11:12:15Z

@sudo-panda @vgvassilev FYI after these changes rootbench (https://github.com/root-project/rootbench) is not compiling anymore:

/data/sftnight/workspace/root-benchmark/BUILDTYPE/Release/COMPILER/default/LABEL/performance-cc8/SPEC/default/rootbench/root/hist/hist/TFormulaGradientBenchmarks.cxx: In function ‘void BM_TFormulaPerf(benchmark::State&, const char*, F&&, bool)’:

/data/sftnight/workspace/root-benchmark/BUILDTYPE/Release/COMPILER/default/LABEL/performance-cc8/SPEC/default/rootbench/root/hist/hist/TFormulaGradientBenchmarks.cxx:30:13: error: ‘GradientStorage’ is not a member of ‘TFormula’
   TFormula::GradientStorage result(Npar);
             ^~~~~~~~~~~~~~~

/data/sftnight/workspace/root-benchmark/BUILDTYPE/Release/COMPILER/default/LABEL/performance-cc8/SPEC/default/rootbench/root/hist/hist/TFormulaGradientBenchmarks.cxx:38:19: error: ‘result’ was not declared in this scope
     eval(h, f, x, result.data());

sudo-panda · 2021-08-23T11:14:33Z

@oshadura I have put up a PR for that a few minutes ago: root-project/rootbench#233. Could you please review it?

oshadura · 2021-08-23T11:18:05Z

wonderful! thanks a lot!

sudo-panda force-pushed the clad-hessian-tformula branch from 776aad0 to 9d12fb4 Compare June 8, 2021 13:35

lmoneta self-assigned this Jun 9, 2021

Axel-Naumann reviewed Jun 14, 2021

View reviewed changes

hist/hist/inc/TFormula.h Outdated Show resolved Hide resolved

Axel-Naumann reviewed Jun 14, 2021

View reviewed changes

hist/hist/inc/TFormula.h Outdated Show resolved Hide resolved

Axel-Naumann reviewed Jun 14, 2021

View reviewed changes

hist/hist/inc/TFormula.h Outdated Show resolved Hide resolved

Axel-Naumann reviewed Jun 14, 2021

View reviewed changes

hist/hist/src/TFormula.cxx Outdated Show resolved Hide resolved

Axel-Naumann reviewed Jun 14, 2021

View reviewed changes

hist/hist/src/TFormula.cxx Outdated Show resolved Hide resolved

vgvassilev mentioned this pull request Jul 28, 2021

[cmake] Fix building ROOT with external llvm. #8766

Merged

sudo-panda force-pushed the clad-hessian-tformula branch from aebde14 to 83b06b5 Compare August 12, 2021 09:48

sudo-panda force-pushed the clad-hessian-tformula branch 3 times, most recently from 76aacc4 to 2a5245b Compare August 15, 2021 21:24

sudo-panda force-pushed the clad-hessian-tformula branch 2 times, most recently from 150796a to 5a90208 Compare August 16, 2021 07:33

sudo-panda marked this pull request as ready for review August 16, 2021 07:34

sudo-panda requested a review from bellenot as a code owner August 16, 2021 07:34

sudo-panda force-pushed the clad-hessian-tformula branch from 5a90208 to f8bcc5c Compare August 17, 2021 09:07

vgvassilev reviewed Aug 17, 2021

View reviewed changes

hist/hist/inc/TFormula.h Show resolved Hide resolved

hist/hist/src/TFormula.cxx Outdated Show resolved Hide resolved

hist/hist/src/TFormula.cxx Outdated Show resolved Hide resolved

sudo-panda force-pushed the clad-hessian-tformula branch from f8bcc5c to 74933b5 Compare August 18, 2021 07:40

vgvassilev reviewed Aug 18, 2021

View reviewed changes

hist/hist/inc/TFormula.h Outdated Show resolved Hide resolved

vgvassilev reviewed Aug 18, 2021

View reviewed changes

hist/hist/src/TFormula.cxx Show resolved Hide resolved

hist/hist/src/TFormula.cxx Outdated Show resolved Hide resolved

lmoneta reviewed Aug 18, 2021

View reviewed changes

hist/hist/inc/TFormula.h Show resolved Hide resolved

hist/hist/test/TFormulaHessianTests.cxx Show resolved Hide resolved

hist/hist/inc/TFormula.h Show resolved Hide resolved

sudo-panda force-pushed the clad-hessian-tformula branch from 74933b5 to e072a32 Compare August 18, 2021 14:55

lmoneta approved these changes Aug 20, 2021

View reviewed changes

hist/hist/inc/TFormula.h Show resolved Hide resolved

[clad] Initial support of hessian calculation in TFormula using clad

1fae3c8

TFormula already supports gradient calculation using clad. This commit extends that support for hessian calculation. Thus the hessian generation is done by clad and TFormula wraps it in a trampoline function.

sudo-panda force-pushed the clad-hessian-tformula branch from e072a32 to 1fae3c8 Compare August 20, 2021 08:00

vgvassilev merged commit 35b5aae into root-project:master Aug 20, 2021

Conversation

sudo-panda commented Jun 8, 2021

Uh oh!

phsft-bot commented Jun 8, 2021

Uh oh!

vgvassilev commented Jun 8, 2021

Uh oh!

phsft-bot commented Jun 8, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vgvassilev commented Aug 12, 2021

Uh oh!

phsft-bot commented Aug 12, 2021

Uh oh!

phsft-bot commented Aug 12, 2021

Errors:

Uh oh!

vgvassilev commented Aug 16, 2021

Uh oh!

phsft-bot commented Aug 16, 2021

Uh oh!

phsft-bot commented Aug 16, 2021

Errors:

Uh oh!

phsft-bot commented Aug 16, 2021

Errors:

Uh oh!

phsft-bot commented Aug 16, 2021

Errors:

Uh oh!

phsft-bot commented Aug 16, 2021

Errors:

Uh oh!

phsft-bot commented Aug 16, 2021

Errors:

Uh oh!

phsft-bot commented Aug 16, 2021

Errors:

Uh oh!

phsft-bot commented Aug 16, 2021

Failing tests:

Uh oh!

sudo-panda commented Aug 17, 2021

Uh oh!

vgvassilev commented Aug 17, 2021

Uh oh!

phsft-bot commented Aug 17, 2021

Uh oh!

phsft-bot commented Aug 17, 2021

Errors:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vgvassilev commented Aug 18, 2021

Uh oh!

phsft-bot commented Aug 18, 2021

Uh oh!

phsft-bot commented Aug 18, 2021

Errors:

Uh oh!

phsft-bot commented Aug 18, 2021

Errors:

Uh oh!

vgvassilev commented Aug 18, 2021

Uh oh!

phsft-bot commented Aug 18, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phsft-bot commented Aug 18, 2021

Errors:

Uh oh!

lmoneta left a comment

Choose a reason for hiding this comment