[RF][PyROOT] Fix `RooDataSet.from_numpy()` for contiguous input arrays by guitargeek · Pull Request #13609 · root-project/root

guitargeek · 2023-09-05T22:02:39Z

phsft-bot · 2023-09-05T22:02:46Z

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac11/noimt, mac12arm/cxx20, windows10/default
How to customize builds

phsft-bot · 2023-09-05T22:26:01Z

Build failed on ROOT-ubuntu2204/nortcxxmod.
Running on root-ubuntu-2204-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

projectroot.tmva.pymva.test.gtest_tmva_pymva_test_TestRModelParserPyTorch

phsft-bot · 2023-09-05T22:36:21Z

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-1.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

projectroot.roottest.python.distrdf.dask.roottest_python_distrdf_dask_test_all

github-actions · 2023-09-05T23:42:38Z

Test Results

      10 files       10 suites 2d 7h 34m 3s ⏱️
  2 484 tests   2 476 ✔️ 0 💤   8 ❌
23 777 runs 23 751 ✔️ 0 💤 26 ❌

For more details on these failures, see this check.

Results for commit bff178d.

♻️ This comment has been updated with latest results.

phsft-bot · 2023-09-06T00:23:06Z

Build failed on mac11/noimt.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

vepadulano

Looks good overall, thanks! Left a couple of minor comments

vepadulano · 2023-09-06T15:02:56Z

-            void_p = ctypes.cast(beg, ctypes.c_voidp).value + 4 * len(arr)
-            end = ctypes.cast(void_p, ctypes.POINTER(ctypes.c_int))
-            ROOT.std.copy(beg, end, vec.begin())
+            # Make sure that the array is contiguous to we can std::copy() it.


Suggested change

# Make sure that the array is contiguous to we can std::copy() it.

# Make sure that the array is contiguous so we can std::copy() it.

vepadulano · 2023-09-06T15:04:35Z

+
+                vec.resize(len(arr))
+
+                beg = arr.ctypes.data_as(ctypes.POINTER(c_type))


Possibly add comment here

Suggested change

beg = arr.ctypes.data_as(ctypes.POINTER(c_type))

# The next part works because arr is guaranteed to be C-contiguous

beg = arr.ctypes.data_as(ctypes.POINTER(c_type))

vepadulano · 2023-09-06T15:05:19Z


+    def test_non_contiguous_arrays(self):
+        """Test whether the import also works with non-continguous arrays.
+        Covers GitHub issue #1360.


Suggested change

Covers GitHub issue #1360.

Covers GitHub issue #13605.

phsft-bot · 2023-09-06T15:38:34Z

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac11/noimt, mac12arm/cxx20, windows10/default
How to customize builds

guitargeek · 2023-09-06T15:39:12Z

Thanks a lot for your review, @vepadulano! I have addressed the comments.

Closes root-project#13605.

phsft-bot · 2023-09-06T15:39:58Z

Build failed on ROOT-ubuntu2204/nortcxxmod.
See console output.

phsft-bot · 2023-09-06T16:41:29Z

Build failed on ROOT-performance-centos8-multicore/soversion.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2023-09-06T16:55:34Z

Build failed on mac11/noimt.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2023-09-06T17:07:24Z

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-3.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2023-09-06T17:07:28Z

Build failed on windows10/default.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

phsft-bot · 2023-09-06T20:49:29Z

Build failed on mac12arm/cxx20.
Running on macphsft26.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

guitargeek added the in:RooFit label Sep 5, 2023

guitargeek requested a review from lmoneta September 5, 2023 22:02

guitargeek requested a review from Axel-Naumann as a code owner September 5, 2023 22:02

guitargeek self-assigned this Sep 5, 2023

guitargeek requested a review from vepadulano as a code owner September 5, 2023 22:02

guitargeek added the in:Python Interface label Sep 5, 2023

guitargeek added the bug label Sep 5, 2023

vepadulano approved these changes Sep 6, 2023

View reviewed changes

guitargeek force-pushed the issue-13605 branch from e8d262a to bff178d Compare September 6, 2023 15:38

[RF][PyROOT] Fix RooDataSet.from_numpy() for contiguous input arrays

bff178d

Closes root-project#13605.

guitargeek merged commit 45230e3 into root-project:master Sep 7, 2023

guitargeek deleted the issue-13605 branch September 7, 2023 12:54

	# Make sure that the array is contiguous to we can std::copy() it.
	# Make sure that the array is contiguous so we can std::copy() it.


		vec.resize(len(arr))

		beg = arr.ctypes.data_as(ctypes.POINTER(c_type))

	beg = arr.ctypes.data_as(ctypes.POINTER(c_type))
	# The next part works because arr is guaranteed to be C-contiguous
	beg = arr.ctypes.data_as(ctypes.POINTER(c_type))

Uh oh!

Conversation

guitargeek commented Sep 5, 2023

Uh oh!

phsft-bot commented Sep 5, 2023

Uh oh!

phsft-bot commented Sep 5, 2023

Failing tests:

Uh oh!

phsft-bot commented Sep 5, 2023

Failing tests:

Uh oh!

github-actions Bot commented Sep 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

phsft-bot commented Sep 6, 2023

Failing tests:

Uh oh!

vepadulano left a comment

Choose a reason for hiding this comment

Uh oh!

vepadulano Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

vepadulano Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

vepadulano Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

phsft-bot commented Sep 6, 2023

Uh oh!

guitargeek commented Sep 6, 2023

Uh oh!

phsft-bot commented Sep 6, 2023

Uh oh!

phsft-bot commented Sep 6, 2023

Failing tests:

Uh oh!

phsft-bot commented Sep 6, 2023

Failing tests:

Uh oh!

phsft-bot commented Sep 6, 2023

Failing tests:

Uh oh!

phsft-bot commented Sep 6, 2023

Failing tests:

Uh oh!

phsft-bot commented Sep 6, 2023

Failing tests:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 5, 2023 •

edited

Loading