Closed
Conversation
If we're not copying data forward, shrink as requested.
Contributor
Author
|
@pcanal - we're testing this in CMS. Will let you know if it went well. |
Member
|
merged into the master. |
Closed
guitargeek
added a commit
to guitargeek/root
that referenced
this pull request
Mar 7, 2026
For small unit tests like the RooFit pythonization tests, the overhead
of starting the Python interpreter is dominant. That means the tests
should not be too granular. I think testing the RooFit pythonizations
with separate Python executions at the level of the different RooFit
classes was indeed too granular. Therefore, this commit suggests to have
a single RooFit pythonization unit test.
Running the unit tests before (single thread):
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection
1/14 Test root-project#64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection ....... Passed 1.09 sec
Start 65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto
2/14 Test root-project#65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto ........ Passed 1.13 sec
Start 66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton
3/14 Test root-project#66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton ...... Passed 1.14 sec
Start 67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist
4/14 Test root-project#67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist ............. Passed 0.93 sec
Start 68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg
5/14 Test root-project#68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg .............. Passed 0.95 sec
Start 69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy
6/14 Test root-project#69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy ...... Passed 1.32 sec
Start 70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton
7/14 Test root-project#70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton ..... Passed 1.09 sec
Start 71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset
8/14 Test root-project#71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset ............. Passed 1.03 sec
Start 72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy
9/14 Test root-project#72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy ....... Passed 1.70 sec
Start 73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc
10/14 Test root-project#73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc .......... Passed 1.21 sec
Start 74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool
11/14 Test root-project#74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool ... Passed 1.40 sec
Start 75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist
12/14 Test root-project#75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist .......... Passed 0.87 sec
Start 76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous
13/14 Test root-project#76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous ........ Passed 1.09 sec
Start 77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace
14/14 Test root-project#77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace ........... Passed 1.21 sec
100% tests passed, 0 tests failed out of 14
Label Time Summary:
python = 16.16 sec*proc (14 tests)
python_runtime_deps = 3.02 sec*proc (2 tests)
Total Test time (real) = 16.17 sec
```
With this commit:
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit
1/1 Test root-project#64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit ... Passed 3.27 sec
100% tests passed, 0 tests failed out of 1
Label Time Summary:
python = 3.27 sec*proc (1 test)
Total Test time (real) = 3.27 sec
```
guitargeek
added a commit
that referenced
this pull request
Mar 7, 2026
For small unit tests like the RooFit pythonization tests, the overhead
of starting the Python interpreter is dominant. That means the tests
should not be too granular. I think testing the RooFit pythonizations
with separate Python executions at the level of the different RooFit
classes was indeed too granular. Therefore, this commit suggests to have
a single RooFit pythonization unit test.
Running the unit tests before (single thread):
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection
1/14 Test #64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection ....... Passed 1.09 sec
Start 65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto
2/14 Test #65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto ........ Passed 1.13 sec
Start 66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton
3/14 Test #66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton ...... Passed 1.14 sec
Start 67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist
4/14 Test #67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist ............. Passed 0.93 sec
Start 68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg
5/14 Test #68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg .............. Passed 0.95 sec
Start 69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy
6/14 Test #69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy ...... Passed 1.32 sec
Start 70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton
7/14 Test #70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton ..... Passed 1.09 sec
Start 71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset
8/14 Test #71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset ............. Passed 1.03 sec
Start 72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy
9/14 Test #72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy ....... Passed 1.70 sec
Start 73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc
10/14 Test #73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc .......... Passed 1.21 sec
Start 74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool
11/14 Test #74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool ... Passed 1.40 sec
Start 75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist
12/14 Test #75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist .......... Passed 0.87 sec
Start 76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous
13/14 Test #76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous ........ Passed 1.09 sec
Start 77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace
14/14 Test #77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace ........... Passed 1.21 sec
100% tests passed, 0 tests failed out of 14
Label Time Summary:
python = 16.16 sec*proc (14 tests)
python_runtime_deps = 3.02 sec*proc (2 tests)
Total Test time (real) = 16.17 sec
```
With this commit:
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit
1/1 Test #64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit ... Passed 3.27 sec
100% tests passed, 0 tests failed out of 1
Label Time Summary:
python = 3.27 sec*proc (1 test)
Total Test time (real) = 3.27 sec
```
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
2 participants
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.
Previous fixes to TBuffer::Expand broke its ability to shrink when data is not being copied (essential for keeping TBranches small).