-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[tmva] Update RBatchGenerator, add tests #15510
Closed
kristupaspranc
wants to merge
653
commits into
root-project:master
from
kristupaspranc:separate_chunking_for_filters
Closed
[tmva] Update RBatchGenerator, add tests #15510
kristupaspranc
wants to merge
653
commits into
root-project:master
from
kristupaspranc:separate_chunking_for_filters
Conversation
This file contains 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
kristupaspranc
requested review from
bellenot,
martamaja10,
vepadulano,
couet,
lmoneta and
dpiparo
as code owners
May 14, 2024 14:36
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit a3dc003. |
In the last build one parenthesis in the file was missing. With the last PR it was fixed and improved the code in other areas where the code was horrible |
In https://root.cern.ch/doc/master/namespaces.html the description of the ROOT namespace was picked form this file brief. Try to fix it this way.
Thanks vepadulano! Co-authored-by: Vincenzo Eduardo Padulano <v.e.padulano@gmail.com>
since the warning is not needed in case the graphics system, cocoa or x11, is OFF. Fixes root-project#15027
Re-implement `MakeNumpyDataFrame` with no interpreter calls, and avoid a explicit lifetime management of Python objects in the RDNumpyDS helper. Like this, the former `RNumpyDS.hxx` helper could be moved to `tree/dataframe`, as it doesn't import `Python.h` anymore. The `RNumpyDS` also got renamed to `RVecDS`, because it takes RVecs and not NumPy arrays directly.
Calling `file->SetBit(TFile::k630forwardCompatibility);` will request to file to store the current value of the bit kIsOnHeap and kNotDeleted for reading in older version that were not setting their value based on the actual state of the read into object
The default is no.
Contributes to root-project#8758 Co-authored-by: Jonas Rembser <jonas.rembser@cern.ch>
The new cppyy has improved memory management with lifelines, so the manual calls to `del` to get the right deletion order are not necessary anymore. Closes root-project#10891.
Otherwise, there is no clear ownership model, which might result in PyROOT crashes at cleanup.
* [RNTuple] describe how exactly `Index` columns are "relative to the cluster" page-by-page follow up of root-project#14949 * mention encoding is page-wise
- Export a couple of missing symbols (x86) - Re-enable several tests working now with LLVM 16 and VS 2022 - Reorganize `llvm13_broken_tests` and `win_broken_tests` for clarity
So far, the signature for the function that is called for the vectorized evaluation was this one: ```c++ void RooAbsReal::computeBatch(double* output, size_t nEvents, RooFit::Detail::DataMap const& dataMap) const ``` This commit is suggesting a new signature: ```c++ void doEval(RooFit::EvalContext & ctx) const; ``` The idea is to make the signature as short as possible, so it doesn't have to be changed anymore if more information needs to be passed. That's why the only parameter is now an `EvalContext` object, reminicint of the old `RunContext` object that fulfilled this task in the very first implementation of the BatchMode by Stephan. The name is now simply `doEval`, because the overloaded term "Batch" should be dropped. It needed to be something with "evaluate", because there is also `RooAbsReal::evaluate()` and we are talking about "evaluation backends". The motivation to change this interface now is because I want to write a documentation for developers (like CMS combine mainteiners) on how to use these new interfaces. And if they start to use it, the interfaces should not change anymore. Than's why I'm doing this change now, which I had in mind already for some time.
The logic that determined the offset hiding of not was coded inside the `RooNLLVarNew` evaluation function so far. This caused trouble, because a change in the global `RooAbsReal::hideOffset()` state did not mark the NLL as dirty. Therefore, it was unpredictable if the offset was actually hidden or not. This commit suggests an improved logic: * Reducer nodes like the NLL always register a value and an offset to the `EvalContext` * The evaluator decides whether to subtract the offset or not * A change in `hideOffset()` makes the evaluator wrapper set all reducer nodes to dirty A new unit test to cover this was also implemented
* [CI] Enable fftw3, sqlite, and xml on Windows * Remove the options matching the ones in `global.txt` (thanks Jonas R.)
These classes are used in the CMS Higgs discovery analysis, that's why we want to support this now.
If such mode is enabled, one tries following: 1. local display like qt5/qt6/cef 2. native displays like chrome/firefox/edge 3. default systeb web browser This mode will be default when root --web specified
Introduce special method to check if real http server required. In case of "on" mode if there are cef or qt6 libs one can avoid creation of http server
Add more info into `root --help` Extend TROOT::SetWebDisplay docu
… lambda recursion
Introduce a new method to get a label for the data source that the current RDataFrame is processing. There are three main types: * The dataframe will process a TTree dataset * The dataframe will process an empty dataset * The dataframe will process data from an RDataSource The function returns a label with the suffix "DS" also for the first two cases, to be aligned as much as possible with the RDataSource infrastructure.
kristupaspranc
force-pushed
the
separate_chunking_for_filters
branch
from
May 21, 2024 13:29
ffc2f6c
to
eaaaa5e
Compare
kristupaspranc
requested review from
linev,
osschar,
jblomer,
pcanal,
guitargeek,
gganis,
vgvassilev and
agheata
as code owners
May 21, 2024 13:29
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fix detected bugs, add separate function for chunk loading with filters, add tests