[TMVA] Fix for using integer spectator types in Reader class#9513
[TMVA] Fix for using integer spectator types in Reader class#9513lmoneta merged 3 commits intoroot-project:masterfrom
Conversation
|
Starting build on |
TMVA::Reader::AddSpectator allows passing an integer external pointer, but the event class stores only float * pointer. Now add in Event class the type information of the spectator variables and the correct casting is performed when retrieving the spectator variable value Note that the integer is always converted to a float, so one can use integer values only from -[2^24, 2^24]. This fixes root-project#9115
…for the spectator's. This test will fail in current master but it passes with the PR root-project#9513
lmoneta
force-pushed
the
fix_tmva_spectators_int
branch
from
21a9a31 to
cac9566
Compare
|
Starting build on |
|
Added a new test for checking that the issue is fixed! |
Axel-Naumann
left a comment
Axel-Naumann
left a comment
There was a problem hiding this comment.
The test looks complex for something that fundamental. Is there no way to load an existing BDT, also to save test time? How much is the running time of this - if it's < 1 second then whatever, but if it's longer just to check whether int and float spectators then maybe we can improve that?
| std::vector<char> spectatorTypes; | ||
| for (; it != spectatorinfos.end(); ++it) { | ||
| evdyn->push_back( (Float_t*)(*it).GetExternalLink() ); | ||
| spectatorTypes.push_back(it->GetVarType()); | ||
| } |
There was a problem hiding this comment.
| std::vector<char> spectatorTypes; | |
| for (; it != spectatorinfos.end(); ++it) { | |
| evdyn->push_back( (Float_t*)(*it).GetExternalLink() ); | |
| spectatorTypes.push_back(it->GetVarType()); | |
| } | |
| std::vector<char> spectatorTypes; | |
| spectatorTypes.reserve(spectatorinfos.size()); | |
| for (auto &&info: spectatorinfos) { | |
| evdyn->push_back( (Float_t*)info.GetExternalLink() ); | |
| spectatorTypes.push_back(info.GetVarType()); | |
| } |
|
|
||
| // AccessPathName() == kFALSE means file exits | ||
| ASSERT_FALSE(gSystem->AccessPathName(weightPath.c_str())) << "Method was" | ||
| << " not serialised correctly. Path: '" << weightPath << "' does not" |
There was a problem hiding this comment.
| << " not serialised correctly. Path: '" << weightPath << "' does not" | |
| << " not serialized correctly. Path: '" << weightPath << "' does not" |
|
|
||
|
|
||
| output[ievt] = reader.EvaluateMVA("BDT1"); | ||
|
|
There was a problem hiding this comment.
| output[ievt] = reader.EvaluateMVA("BDT1"); | |
| output[ievt] = reader.EvaluateMVA("BDT1"); |
|
Thank you Axel for the review comments ! I will include them. |
|
Starting build on |
|
Build failed on ROOT-debian10-i386/cxx14. |
|
Build failed on ROOT-performance-centos8-multicore/default. |
|
Build failed on mac11/cxx17. |
|
Build failed on windows10/cxx14. |
|
Build failed on ROOT-ubuntu16/nortcxxmod. |
|
Build failed on ROOT-ubuntu2004/soversion. |
|
Build failed on mac1015/python3. |
…for the spectator's. This test will fail in current master but it passes with the PR #9513
TMVA::Reader::AddSpectator allows passing an integer external pointer, but the event class stores only float * pointer. Now add in Event class the type information of the spectator variables and the correct casting is performed when retrieving the spectator variable value Note that the integer is always converted to a float, so one can use integer values only from -[2^24, 2^24]. This fixes root-project#9115 Add a test for checking using an integer variable in the TMVA Reader for the spectator's. This test will fail in current master but it passes with the PR root-project#9513 Implement review comments
TMVA::Reader::AddSpectator allows passing an integer external pointer, but the event class stores only float * pointer. Now add in Event class the type information of the spectator variables and the correct casting is performed when retrieving the spectator variable value Note that the integer is always converted to a float, so one can use integer values only from -[2^24, 2^24]. This fixes #9115 Add a test for checking using an integer variable in the TMVA Reader for the spectator's. This test will fail in current master but it passes with the PR #9513 Implement review comments
3 participants
TMVA::Reader::AddSpectator allows passing an integer external pointer, but the event class stores only float * pointer.
This PR adds in the Event class the type information of the spectator variables. With this the correct casting is performed when retrieving the spectator variable value in case of dynamic events (the inputs are provided using an external pointer)
Note that the integer is always converted to a float, so one can use integer values only from [-2^24, 2^24].
This PR fixes #9115