Max pool #18
Open
Max pool #18
Conversation
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
Such pointer will be automatically released when unloading library or program is stopped
In commit 196ef3b, `operator=` for the RooPolyFunc was implemented, but it was implemented wrognly: it used the assignment operator the the proxy collections, which does value syncronization and not copy assignment! It's better to delete the copy assignment for RooPolyFunc, because copy/move assignment of RooFit objects is very badly supported.
There was a bug in a schema evolution rule for a very old RooProduct version, where a `RooSetProxy` member was replaced by a `RooListProxy`. The problem was that only the content was copied to the new proxy list, not the other members. This lead to an invalid state for the RooSetProxy, because it had no registered owner. Commit aa55e5c attempted to fix the problem, but the fix used the `RooArgList::operator=`, which doesn't implement copy assignment, but value synchronization! So the target proxy list was empty after calling it. Therefore, one the RooCollectionProxy needs to be extended with something that does exactly what the schema evolution requires: setting the owner of the proxy and adding all elements without registering the elements as servers (the server lists are copied separately by IO). A new member function initializeAfterIOConstructor` was added to the RooCollectionProxy for this. In the initial attempt to the fix, it was thought the original problem was only that the proxy owner was not set in the proxy, but actually the _proxyList only contained `nullptr` after reading and old RooProduct! The reason for that is the reason for which `RooAbsArg::ioStreamerPass2()` exists (quoting from its documentation): > second pass is typically needed when evolving data member of > RooAbsArg-derived classes that are container classes with references > to other members, which may not yet be 'live' in the first > ioStreamer() evolution pass. Hence, `RooProduct` needs to override `ioStreamerPass2()` and set the proxy list correctly there, which is also done in this commit.
The documentation of `RooAbsArg::ioStreamerPass2()` says: > Classes may overload this function, but must call the base method in > the overloaded call to ensure base evolution is handled properly This was not done in the overload in RooHistFunc, which is a mistake that probably leads to backwards compatibility errors that are undetected to far.
Two of the previous commits have shown that copy assignment for RooCollectionProxy is dangerous because it behaves unexpectedly. However, it is not clear how it *should* behave. Should default assignment be used? But then, it will use the assignment operators of the RooFit collections, which actually don't do assignment, but value syncronization! Should it be re-implemented to be actual assignment? That would be inconsistent with the base class! So it's better to not support it at all.
This commit renames and refactors `RDaosEventQueue` to include: - Parent events that are responsible for a batch of asynchronous events originating from a single RW call. This simplifies polling, which is a waiting call on the batch of requests sent under that parent that are inflight (`WaitOnParentBarrier`, which sets a parent event barrier and waits for all children launched before it to complete). - Poolwide event queues that are shared among its underlying containers and persist between RW calls. This eliminates the significant overhead behind instantiating new queues for each call to `VectorReadWrite`. - Queue management obeying RAII principle. - Better handling of synchronous events in `FetchUpdateArgs` (the default behavior). The event is an optional attribute which yields a nullptr to DAOS fetch/update calls in such cases, indicating the request is blocking. - The functions `daos_event_test` and `daos_event_parent_barrier`, used for polling and sending parent events respectively, to `libdaos_mock`. Co-authored-by: Giovanna Lazzari Miotto <giovanna.lazzari.miotto@cern.ch> Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>
Ensure that buffer will be deleted when painter is deleted
All contour plots was not releasing that painter at the end
Ensure that allocated memory always released
It is the only workaround to support I/O of TGaxis. If TGaxis was configured from histogram axis and stored, it may store `fModLabs` list from TAxis. After reading back such object from the file one has no information about ownership. Therefore now only lists with configured ownership are deleted - making no crash with old ROOT files.
As per C++ core guidelines.
Before this patch, we were iterating until the end of the map rather than until the end of the equal_range (this was not a problem in practice because program logic guaranteed that we would find what we were looking for and break out of the loop early).
Simplifies memory management
Avoid direct dynamic allocation of plain C arrays
Temporary TBox object was not deleted, now just in stack
Temporary TText object was not deleted, now used in stack
It is good practice to cleanup memory at macro exit
Original object was not deleted properly
This is to get rid of deprecation warninigs when building on Ubuntu 22.04. Should cause no backwards compatibility poblems, as the functions that are used now are around at least since OpenSSL 1.0.2: https://www.openssl.org/docs/man1.0.2/man3/EVP_DigestInit_ex.html This patch was already applied to upstream civet: civetweb/civetweb#1072
Keep old code for Windows and older SSL versions
…d and 3d case (root-project#10768) * Fix the issue related to 1d and 3d MaxPool operators * Update ROperator_Pool.hxx * The issue related to Maxpool for 1d and 3d fixed * Test related to MaxPool 1d added * MaxPool Operator fixed for 1d and 3d cases and tests added * Max Pool operator errors related to 1d and 3d case resolved * Max Pool operator errors related to 1d and 3d case resolved * Updated the Pool Operator * Fix warning in new implementation of Pool operator * Added the tests for MaxPool 2d and 3d Operators * Fix 2d and 3d MaxPool operators Use the correct stride for the height when loopoing at the tensor element. The height stride for 2d is equal to wsize, and in 3d is equal to wsize*dsize. Precompute the hstride in 3d to avoid a multiplication when looping through the tensor elements. * Tests added for AvgPool * Corrected the spelling errors Co-authored-by: moneta <lorenzo.moneta@cern.ch>
lmoneta
pushed a commit
that referenced
this pull request
Feb 18, 2025
526/1416 Test root-project#63: pyunittests-bindings-pyroot-pythonizations-pyroot-pyz-tf-pycallables ..........................***Failed 75.51 sec test_callable (tf_pycallables.TF1.test_callable) Test function provided as callable ... /usr/include/c++/15/bits/stl_bvector.h:1134: std::vector<bool, _Alloc>::reference std::vector<bool, _Alloc>::operator[](size_type) [with _Alloc = std::allocator<bool>; reference = std::vector<bool>::reference; size_type = long unsigned int]: Assertion '__n < this->size()' failed. *** Break *** abort =========================================================== The lines below might hint at the cause of the crash. If you see question marks as part of the stack trace, try to recompile with debugging information enabled and export CLING_DEBUG=1 environment variable before running. You may get help by asking at the ROOT forum https://root.cern/forum preferably using the command (.forum bug) in the ROOT prompt. Only if you are really convinced it is a bug in ROOT then please submit a report at https://root.cern/bugs or (preferably) using the command (.gh bug) in the ROOT prompt. Please post the ENTIRE stack trace from above as an attachment in addition to anything else that might help us fixing this issue. =========================================================== #10 0x00007f6bcac7fea4 in __pthread_kill_implementation () from /lib64/libc.so.6 #11 0x00007f6bcac264de in raise () from /lib64/libc.so.6 #12 0x00007f6bcac0e350 in abort () from /lib64/libc.so.6 #13 0x00007f6bc9a0be02 in std::__glibcxx_assert_fail(char const*, int, char const*, char const*) () from /lib64/libstdc++.so.6 #14 0x00007f6bb9b15257 in std::vector<bool, std::allocator<bool> >::operator[](unsigned long) [clone .part.0] [clone .lto_priv.0] (__n=<optimized out>, this=<optimized out>) at /usr/include/c++/15/bits/stl_bvector.h:1134 #15 0x00007f6bb9bab976 in std::vector<bool, std::allocator<bool> >::operator[] (this=<synthetic pointer>, __n=0) at /usr/include/c++/15/bits/stl_bvector.h:201 #16 CPyCppyy::Utility::ConstructCallbackPreamble (retType="Double_t", argtypes=..., code=Python Exception <class 'IndexError'>: list index out of range #17 0x00007f6bb9b389e0 in PyFunction_AsCPointer (pyobject=<optimized out>, pyobject entry=0x7f6bb8f838c0, rettype="Double_t", signature="(Double_t*,Double_t*)") at /builddir/build/BUILD/root-6.34.02-build/root-6.34.02/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx:2713 #18 0x00007f6bb9b39851 in CPyCppyy::(anonymous namespace)::FunctionPointerConverter::SetArg (this=0x55eb25d61d40, pyobject=0x7f6bb8f838c0, para=..., ctxt=0x7ffc484855c0) at /builddir/build/BUILD/root-6.34.02-build/root-6.34.02/bindings/pyroot/cppyy/CPyCppyy/src/Converters.cxx:2768
lmoneta
pushed a commit
that referenced
this pull request
Apr 3, 2025
This fixes root-project#18236 As seen in cms-sw/cmssw#47763 (comment) there are cases where the ROOT global lock is taken too late: In the stack trace below the lock is requested on frame #7 where as it should really be (also) requested on frame #12 as soon as the transaction is being generated. ``` #6 0x00001501421f55ea in ROOT::TVirtualRWMutex::Lock() () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCore.so #7 0x0000150133a680ca in TCling::HandleNewTransaction(cling::Transaction const&) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCling.so #8 0x0000150133a85778 in TCling::UpdateListsOnCommitted(cling::Transaction const&) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCling.so #9 0x0000150133b60018 in cling::MultiplexInterpreterCallbacks::TransactionCommitted(cling::Transaction const&) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCling.so #10 0x0000150133bec069 in cling::IncrementalParser::commitTransaction(llvm::PointerIntPair<cling::Transaction*, 2u, cling::IncrementalParser::EParseResult, llvm::PointerLikeTypeTraits<cling::Transaction*>, llvm::PointerIntPairInfo<cling::Transaction*, 2u, llvm::PointerLikeTypeTraits<cling::Transaction*> > >&, bool) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCling.so #11 0x0000150133b5663a in cling::Interpreter::PushTransactionRAII::~PushTransactionRAII() () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCling.so #12 0x0000150133b6b883 in cling::LookupHelper::findType(llvm::StringRef, cling::LookupHelper::DiagSetting) const () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCling.so #13 0x00001501339b6b06 in ROOT::TMetaUtils::TClingLookupHelper::GetPartiallyDesugaredNameWithScopeHandling(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, bool) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCling.so #14 0x00001501422f1458 in ResolveTypedefProcessType(char const*, unsigned int, unsigned int, bool, unsigned int, unsigned int, unsigned int, bool&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) [clone .constprop.0] () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCore.so #15 0x00001501422f1a0f in ResolveTypedefImpl(char const*, unsigned int, unsigned int&, bool&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCore.so #16 0x00001501422f2ef3 in TClassEdit::ResolveTypedef[abi:cxx11](char const*, bool) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCore.so #17 0x00001501422f5c20 in TClassEdit::TSplitType::ShortType(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, int) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCore.so #18 0x00001501422f6888 in TClassEdit::GetNormalizedName(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::basic_string_view<char, std::char_traits<char> >) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCore.so #19 0x000015014232e795 in ROOT::Internal::GetDemangledTypeName[abi:cxx11](std::type_info const&) () from /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02883/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_15_1_RNTUPLE_X_2025-03-31-2300/external/el8_amd64_gcc12/lib/libCore.so ```
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.
This Pull request:
Changes or fixes:
Checklist:
This PR fixes #