Geometry Python Tutorials #15564
Closed
Geometry Python Tutorials #15564
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
Don't check the Minos output code, because if the Minos errors are at the limit, Minuit2 will return nonzero, unlike Minuit 1. This case where the errors are at limit should not cause a failure though. The `stressRooStats` test suite actively provokes it, by fitting a dataset with a single observed value that corresponds to a best-fit parameter value at the limit. With these changes, the `stressRooStats` suite will also pass with Minuit2 and BatchMode, which is now added as a unit test.
Since Python 3.12, in the implementation of 'classobject.h' the function
PyMethod_GET_SELF performs an assert to check that the passed function argument
is a method:
```
\#define _PyMethod_CAST(meth) \
(assert(PyMethod_Check(meth)), _Py_CAST(PyMethodObject*, meth))
[...]
static inline PyObject* PyMethod_GET_SELF(PyObject *meth) {
return _PyMethod_CAST(meth)->im_self;
}
```
It's fair that the assert fails, because the Python type of `meth` in
this context is not a `PyMethod_Type`, but the
`CustomInstanceMethod_Type` from cppyy. However, as can be seen in the
implementation of `CustomInstanceMethod_New`, the actual C++ type that
implements this custom cppy type is just the regular `PyMethodObject`.
Hence, this commit suggests new assert-free `CustomInstanceMethod_GET_*`
macros that replace the `PyMethod_GET_*` macros in the context of the
`CustomInstanceMethod` implementation.
This allows us to better control the test parameters in CMake with common RooFit configuration varaibles, without having to "leak" this information outside the RooFit build configuration. For example, we now don't run the stress tests with the legacy evaluation backend if RooFit is not built with it.
The `R__HAS_ROOFIT_MULTIPROCESS` is not used in any public header files, so it's not needed. In the RooFit implementation, it is replaced with a `ROOFIT_MULTIPROCESS` define that is part of the RooFitCore target. The `R__HAS_ROOFIT_MULTIPROCESS` was only introduced in ROOT 6.26 and it was never advocated to users, so it should be safe to remove it.
The `roofitmore` is only compiled anyway if mathmore is available.
In RooFit, there are checks if ROOT is build with CUDA only in the `.cxx` implementation files, not in the public headers. We therefore don't need a `R__HAS_CUDA` in the ROOT configuration. The `R__HAS_CUDA` flag was only introduced in ROOT 6.26 and it was never advocated to users, so it should be safe to remove it.
This commit enables serialization of the Python proxy defined within cppyy to the C++ std::runtime_error class. By doing so, we avoid errors that happen in certain configurations of Dask tasks where a std::runtime_error may be thrown within the event loop itself. Recent CI failures report the following: ``` RDataFrame::Run: event loop was interrupted Warning in <TBufferFile::WriteObjectAny>: since runtime_error has no public constructor which can be called without argument, objects of this class can not be read with the current library. You will need to add a default constructor before attempting to read it. Warning in <TStreamerInfo::Build>: runtime_error: base class exception has no streamer or dictionary it will not be saved Warning in <TStreamerInfo::Build>: runtime_error: __cow_string has no streamer or dictionary, data member "_M_msg" will not be saved Error in <TClass::New>: cannot create object of class runtime_error Error in <TBufferFile::ReadObject>: could not create object of class runtime_error *** Break *** segmentation violation ``` This is due to the fact that for some reason the std::runtime_error object is serialized at the end of the function scope within the Dask task. The culprit is the [tblib](https://github.com/ionelmc/python-tblib) Python package, a dependency of Dask. Its most recent version (3.0.0) released on October 22th changes the pickling behaviour of exception objects, which is now evidently more enforced than before. Downgrading to previous versions of tblib make the errors above disappear in the distributed RDataFrame tests.
TGraph2DPainter keep pointers on TGraph2D data, but was not checking if these values were changed. Fixes problem reported https://root-forum.cern.ch/t/56816/
When searching for a new point between p1 and p2, if MnCross fails, retry using a position closer to p1 instead of the middle. Thsi is what is done in old TMinuit. Minuit2 was trying to switch direction of the search and this was causing finding points in the opposite side of contour and messing up the order. Improve also debugging of MnContour
The computed value of the Minos error was WRONG when parameter+error is over the limit, but the minos point is before the limit.
Thsi can happen for example when computing a limit on a fraction parameter that is bound to be less than 1.
…en g2 is zero When g2 is zero better using an arbitrary value of 1 in the covariance then a very large one, which can be problematic. This commit restores what is so far used in the legacy TMinuit
…s are not present In this case the limit value is zero, so a special treatment is needed to check if a limit is present or not Add also suggestion by Jonas review
After previous commits finding the contour works now in Minuit2
…ed covariance Use precision.Eps() instead of precision.Eps2() for threshold for the lower values for the initial seed covariance matrix
ReadObj returns an owning pointer. Using dynamic_cast and ignoring the "not castable" case can leak memory. See: https://root-forum.cern.ch/t/possible-leak-with-dynamic-cast-and-tkey-readobj/56799
* Correct code highlighting in doxygen * Fix some references to other functions and classes in the reference guide * Add a section that summarizes the relevent implementation details for RooFit AD
In particular, make sure that the brief class description is not too verbose and does not repeat the class name.
For his work on the RooFit Developer Documentation.
While histogram not stored with TGraph2D, it should be append to the pad list of primitives and drawn independently
Otherwise TF1::Save store wrong values, which difficult to use
In some old examples "same" and "sames" draw options for TF1 was used. Currently if "sames" used for TF1 - plain histogram painting is performed which is wrong for the function. Also improve PMC/PLC/PFC handling
1. Fix - proper fit pars display in stats, proper #chi^{2}
2. Fix - several bugs in TFormula parsing
3. Fix - correctly use saved buffer in TF1/TF2
In "classical" TBrowser one want to use normal TCanvas - even if rootrc configured differently. Therefore change gEnv values for the time when canvas is created
* Fix Cocoa GUI for MacOS14 and clang5 * Fix the text editor (Thanks Timur !) (cherry picked from commit 6593d77)
This fixes using the fit2dHist.C when using the global fit (user defined FCN) This PR fixes root-project#13906
Fails sporadically, root-project#14121 (cherry picked from commit 5560b67)
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.
PPaye
requested review from
bellenot,
linev,
osschar,
couet,
martamaja10,
vepadulano,
pcanal,
lmoneta,
guitargeek,
gganis,
dpiparo,
vgvassilev and
agheata
as code owners
agheata
approved these changes
May 20, 2024
|
Hi @PPaye , there is a problem with this PR. Could you perhaps rebase it to ROOT master? |
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:
Adds a Complete translation of the Geometry-Tutorials into python.
Changes or fixes:
It runs, it re-reruns, and it gets out of IPython without crashing memory.
The Root-Objects have been cautiously removed once they have been created by
pyroot, this helps to run the scripts again-and-again without crashing memory, since ROOT hasn't the ownership of the Objects anymore.Enjoy the
pygeomtutorials, if so.Checklist:
This PR fixes #
It doesn't fix anything but adds alternatives approach to use
pyrootwiththe Geometry Package.