Skip to content

[tmva] Use TDecompChol in GetSQRootMatrix. From Eddy! - #9996

Merged
guitargeek merged 1 commit into
root-project:masterfrom
Axel-Naumann:tmva-sqrootmat2
Dec 5, 2025
Merged

[tmva] Use TDecompChol in GetSQRootMatrix. From Eddy!#9996
guitargeek merged 1 commit into
root-project:masterfrom
Axel-Naumann:tmva-sqrootmat2

Conversation

@Axel-Naumann

@Axel-Naumann Axel-Naumann commented Feb 28, 2022

Copy link
Copy Markdown
Member

This Pull request:

Changes or fixes:

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #

@Axel-Naumann
Axel-Naumann requested a review from lmoneta February 28, 2022 14:46
@phsft-bot

phsft-bot commented Feb 28, 2022

Copy link
Copy Markdown

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

@Axel-Naumann
Axel-Naumann removed the request for review from sitongan February 28, 2022 14:46
@phsft-bot

phsft-bot commented Feb 28, 2022

Copy link
Copy Markdown

Build failed on ROOT-ubuntu2004/soversion.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

lmoneta
lmoneta previously requested changes Mar 1, 2022
View reviewed changes

@lmoneta lmoneta left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you Eddy for this improvement.
I think it would be nice to have a test which can show that the code is numerically more stable than before

Comment thread tmva/tmva/src/Tools.cxx Resolved
Comment thread tmva/tmva/src/Tools.cxx Resolved
@github-actions

github-actions Bot commented Dec 5, 2025

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 19h 50m 15s ⏱️
 3 783 tests  3 783 ✅ 0 💤 0 ❌
81 226 runs  81 226 ✅ 0 💤 0 ❌

Results for commit 934afc5.

@guitargeek

guitargeek commented Dec 5, 2025

Copy link
Copy Markdown
Contributor

I think it would be nice to have a test which can show that the code is numerically more stable than before

I guess Eddy didn't have time for that. Either way, we should not hold off these numerical improvements!

guitargeek
guitargeek approved these changes Dec 5, 2025
View reviewed changes

@guitargeek guitargeek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this contribution!

@guitargeek
guitargeek requested a review from lmoneta December 5, 2025 11:56
@guitargeek
guitargeek dismissed lmoneta’s stale review December 5, 2025 11:57

Original PR author did not have the time to provide tests.

@guitargeek
guitargeek merged commit 0d1dc18 into root-project:master Dec 5, 2025
31 of 32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@guitargeek guitargeek guitargeek approved these changes
@lmoneta lmoneta Awaiting requested review from lmoneta lmoneta is a code owner

Assignees

@lmoneta lmoneta

Labels

in:TMVA

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Axel-Naumann @phsft-bot @guitargeek @lmoneta