Skip to content
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

rebasing roobspline branch to master #25

Closed
wants to merge 2,822 commits into from
Closed

Conversation

vincecr0ft
Copy link
Member

This Pull request:

Changes or fixes:

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #

linev and others added 30 commits September 5, 2023 17:33
As in ROOT, stats will be drawn also when `AXIS` draw option specified.

Also support `inspectN` draw option for expanding
objects to N level
Extend the regex passed to 'compose_method' to include 'E*(low,high)[d]' getters.
Go back to use .h5 files instead of .keras due to a problem in Keras loading .keras files on MacOS ARM (see https://github.com/keras-team/keras/issues/18278 )
Disbaling it causes an interference with multiprocessing and a restart of the process after it is endeded. This fixes the timeout of the PyKeras tests on MacOS.

This commits apply also some small fixes/improvements  on the deletetion of some TMVA objects.
To make sure the interpreter mutexes exist before locks are needed by
cppyy calls.
Axel-Naumann and others added 29 commits October 1, 2023 11:59
This prevents TROOT.cxx from being rebuilt everytime the commit changes.
Instead, use gitinfo.txt
This isn't needed at compiletime; using the text file prevents rebuilding /
having to maintain `RGitCommit.h`.
Deprecate the use of `RGitCommit.h`: it triggers rebuilds where the git info
is in fact not needed at compiletime; all uses are replaced by parsing
`gitinfo.txt`.

Instead, create `etc/gitinfo.txt` even if the source directory is not a git
repo. This only works for releases (even `ROOT_PATCH_VERSION`), where the
tag and the branch can be determined from the version numbers.
Generalize the arithmetic operator overloads of TComplex to general
arithmetic types with templates.

I'm using some `enable_if` for arithmetic types only, so we can be sure
that nothing unexpected will happen for non-arithmetic types that
implement arithmetic operators with `double` themselves.

Closes root-project#13730.
* Fix potential crash of the rreader test on Windows

Delete the `TFile` pointers, preventing a potential crash in `TROOT::CloseFiles()` when trying to call the `Close()` method on `TWebSocket`/`TWebFile` via the interpreter `CallFunc_Exec` on Windows (visble with LLVM 16)

* Use `std::unique_ptr` instead of deleting the pointers (thanks Jonas)
This is needed to properly close and delete the socket when closing the file in `TROOT::CloseFiles()`, preventing a potential issue when trying to close the socket afterwards
Otherwise they are emitted as internal and we get double-construction
and -destruction on the same memory address due to the way we promote
internal declarations in KeepLocalGVPass.

According to upstream tests, the de-duplication doesn't work on Windows
(yet), but I think this problem is severe enough to fix it on the other
platforms sooner rather than later.

Fixes root-project#13429
Public interface creating an interpreter transaction, needs locking.
* Change name of the RLoopManager data member that stores callbacks registered via RResultPtr::OnPartialResult.
* Be more specific in the docs.
Sample callbacks can be registered by an RAction or an RDefinePerSample
instance. In both cases, the lifetime of the callback is tied to the lifetime of
the object itself. Avoid eager clearing of the callbacks so to not interfer with
the normal functioning.
This is a reproducer test for some sporadic CI failures, e.g.

```python
========================================================================== FAILURES ===========================================================================
_______________________________________________________ TestDefinePerSample.test_definepersample_simple _______________________________________________________

self = <check_definepersample.TestDefinePerSample object at 0x13e0c6190>, connection = <Client: 'tcp://127.0.0.1:55253' processes=2 threads=2, memory=4.00 GiB>

    def test_definepersample_simple(self, connection):
        """
        Test DefinePerSample operation on three samples using a predefined
        string of operations.
        """

        df = Dask.RDataFrame(self.maintreename, self.filenames, daskclient=connection)

        # Associate a number to each sample
        definepersample_code = """
        if(rdfsampleinfo_.Contains(\"{}\")) return 1;
        else if (rdfsampleinfo_.Contains(\"{}\")) return 2;
        else if (rdfsampleinfo_.Contains(\"{}\")) return 3;
        else return 0;
        """.format(*self.samples)

        df1 = df.DefinePerSample("sampleid", definepersample_code)

        # Filter by the sample number. Each filtered dataframe should contain
        # 10 entries, equal to the number of entries per sample
        samplescounts = [df1.Filter("sampleid == {}".format(id)).Count() for id in [1, 2, 3]]

        for count in samplescounts:
>           assert count.GetValue() == 10
E           AssertionError

check_definepersample.py:62: AssertionError
-------------------------------------------------------------------- Captured stderr setup --------------------------------------------------------------------
RDataFrame::Run: event loop was interrupted
2023-09-08 14:51:57,002 - distributed.worker - WARNING - Compute Failed
Key:       dask_mapper-a92ac090-9407-4849-921a-d187ceffd3ed
Function:  dask_mapper
args:      (EmptySourceRange(exec_id=ExecutionIdentifier(rdf_uuid=UUID('5d67c0a7-58f4-488d-8e44-bb5aa0fac480'), graph_uuid=UUID('69353465-0a90-4eef-b101-a1eb93f0c13a')), id=0, start=0, end=50))
kwargs:    {}
Exception: "RuntimeError('C++ exception thrown:\\n\\truntime_error: Graph was applied to a mix of scalar values and collections. This is not supported.')"
```

Which is due to Dask assigning two tasks to the same worker for the test with
the DefinePeSample calls. The Count operation would fail to report the correct
amount of entries due to the fact that the DefinePerSample callback was
previously deleted at the end of every event loop, specifically at the end of
the first task's event loop. Consequently, when the second task starts and it
picks up the same RDataFrame to clone the action, the DefinePerSample would
never be actually called.
@vincecr0ft vincecr0ft closed this Nov 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.