You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/12/30 13:25:44 UTC
[GitHub] [arrow] pitrou opened a new pull request #12054: ARROW-15219: [Python] Export the random compute function
pitrou opened a new pull request #12054:
URL: https://github.com/apache/arrow/pull/12054
Also change the underlying generator as successive seeds (e.g. 0, 1, 2, 3) could produce the same output.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #12054:
URL: https://github.com/apache/arrow/pull/12054
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#discussion_r776838234
##########
File path: python/pyarrow/_compute.pyx
##########
@@ -1331,6 +1331,32 @@ class Utf8NormalizeOptions(_Utf8NormalizeOptions):
self._set_options(form)
+cdef class _RandomOptions(FunctionOptions):
+ def _set_options(self, length, initializer):
+ if initializer == 'system':
+ self.wrapped.reset(new CRandomOptions(
+ CRandomOptions.FromSystemRandom(length)))
+ return
+
+ if not isinstance(initializer, int):
+ try:
+ initializer = hash(initializer)
+ except TypeError:
+ raise TypeError(
+ f"initializer should be 'system', an integer, "
+ f"or a hashable object; got {initializer!r}")
Review comment:
No, because Python automatically propagates the context. `raise from` adds a `__cause__`, which isn't very useful in practice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#issuecomment-1003028663
cc @asuhan
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#issuecomment-1003028435
https://issues.apache.org/jira/browse/ARROW-15219
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#issuecomment-1003155920
Benchmark runs are scheduled for baseline = ceaed97f010b4e1c67a34f73b683b1ca3df16930 and contender = d0628b7ae3a732ce03af47653e9ccddf090d1672. d0628b7ae3a732ce03af47653e9ccddf090d1672 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/6fb78d1503644ad58de18fc868482ac6...d67f135fccf04daaa7f0d0c85ec63f77/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/9dd9c6cca28c4d969dc9aeb9683c8a37...c35a72ba073f442fae496fe3205b0537/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/90ac133856524158b8ea825e76da6272...20ff7b93581540bebe1ecb8f75ba46f3/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] edponce commented on a change in pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
edponce commented on a change in pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#discussion_r776797179
##########
File path: python/pyarrow/_compute.pyx
##########
@@ -1331,6 +1331,32 @@ class Utf8NormalizeOptions(_Utf8NormalizeOptions):
self._set_options(form)
+cdef class _RandomOptions(FunctionOptions):
+ def _set_options(self, length, initializer):
+ if initializer == 'system':
+ self.wrapped.reset(new CRandomOptions(
+ CRandomOptions.FromSystemRandom(length)))
+ return
+
+ if not isinstance(initializer, int):
+ try:
+ initializer = hash(initializer)
+ except TypeError:
+ raise TypeError(
+ f"initializer should be 'system', an integer, "
+ f"or a hashable object; got {initializer!r}")
Review comment:
Nit: Only the bottom string needs formatting. Similarly, in an error message below. Maybe you have it like this for symmetry or text alignment ;)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#issuecomment-1003155920
Benchmark runs are scheduled for baseline = ceaed97f010b4e1c67a34f73b683b1ca3df16930 and contender = d0628b7ae3a732ce03af47653e9ccddf090d1672. d0628b7ae3a732ce03af47653e9ccddf090d1672 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/6fb78d1503644ad58de18fc868482ac6...d67f135fccf04daaa7f0d0c85ec63f77/)
[Failed :arrow_down:1.79% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/9dd9c6cca28c4d969dc9aeb9683c8a37...c35a72ba073f442fae496fe3205b0537/)
[Failed :arrow_down:0.49% :arrow_up:0.13%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/90ac133856524158b8ea825e76da6272...20ff7b93581540bebe1ecb8f75ba46f3/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#discussion_r776808871
##########
File path: python/pyarrow/_compute.pyx
##########
@@ -1331,6 +1331,32 @@ class Utf8NormalizeOptions(_Utf8NormalizeOptions):
self._set_options(form)
+cdef class _RandomOptions(FunctionOptions):
+ def _set_options(self, length, initializer):
+ if initializer == 'system':
+ self.wrapped.reset(new CRandomOptions(
+ CRandomOptions.FromSystemRandom(length)))
+ return
+
+ if not isinstance(initializer, int):
+ try:
+ initializer = hash(initializer)
+ except TypeError:
+ raise TypeError(
+ f"initializer should be 'system', an integer, "
+ f"or a hashable object; got {initializer!r}")
Review comment:
Yes, and it's also less error-prone if we add some formatting to the top string.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#issuecomment-1003099900
Also cc @lidavidm in case you want to take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#issuecomment-1003155920
Benchmark runs are scheduled for baseline = ceaed97f010b4e1c67a34f73b683b1ca3df16930 and contender = d0628b7ae3a732ce03af47653e9ccddf090d1672. d0628b7ae3a732ce03af47653e9ccddf090d1672 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/6fb78d1503644ad58de18fc868482ac6...d67f135fccf04daaa7f0d0c85ec63f77/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/9dd9c6cca28c4d969dc9aeb9683c8a37...c35a72ba073f442fae496fe3205b0537/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/90ac133856524158b8ea825e76da6272...20ff7b93581540bebe1ecb8f75ba46f3/)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#issuecomment-1003030568
Note that this produces a moderate performance regression, but avoiding fragile seed behaviour seems important:
```
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Non-regressions: (2)
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
benchmark baseline contender change % counters
RandomKernelSeed/1 1.237M items/sec 1.191M items/sec -3.779 {'family_index': 1, 'per_family_instance_index': 0, 'run_name': 'RandomKernelSeed/1', 'repetitions': 3, 'repetition_index': 2, 'threads': 1, 'iterations': 854391}
RandomKernelSystem/1 1.208M items/sec 1.161M items/sec -3.875 {'family_index': 0, 'per_family_instance_index': 0, 'run_name': 'RandomKernelSystem/1', 'repetitions': 3, 'repetition_index': 2, 'threads': 1, 'iterations': 832333}
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Regressions: (6)
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
benchmark baseline contender change % counters
RandomKernelSystem/64 70.753M items/sec 66.930M items/sec -5.403 {'family_index': 0, 'per_family_instance_index': 1, 'run_name': 'RandomKernelSystem/64', 'repetitions': 3, 'repetition_index': 2, 'threads': 1, 'iterations': 761829}
RandomKernelSeed/64 72.414M items/sec 68.325M items/sec -5.647 {'family_index': 1, 'per_family_instance_index': 1, 'run_name': 'RandomKernelSeed/64', 'repetitions': 3, 'repetition_index': 0, 'threads': 1, 'iterations': 780859}
RandomKernelSystem/1024 464.748M items/sec 405.753M items/sec -12.694 {'family_index': 0, 'per_family_instance_index': 2, 'run_name': 'RandomKernelSystem/1024', 'repetitions': 3, 'repetition_index': 2, 'threads': 1, 'iterations': 318463}
RandomKernelSeed/1024 469.965M items/sec 409.407M items/sec -12.886 {'family_index': 1, 'per_family_instance_index': 2, 'run_name': 'RandomKernelSeed/1024', 'repetitions': 3, 'repetition_index': 1, 'threads': 1, 'iterations': 320469}
RandomKernelSeed/65536 742.471M items/sec 612.071M items/sec -17.563 {'family_index': 1, 'per_family_instance_index': 3, 'run_name': 'RandomKernelSeed/65536', 'repetitions': 3, 'repetition_index': 0, 'threads': 1, 'iterations': 7800}
RandomKernelSystem/65536 742.684M items/sec 609.872M items/sec -17.883 {'family_index': 0, 'per_family_instance_index': 3, 'run_name': 'RandomKernelSystem/65536', 'repetitions': 3, 'repetition_index': 0, 'threads': 1, 'iterations': 7886}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] lidavidm commented on a change in pull request #12054: ARROW-15219: [Python] Export the random compute function
Posted by GitBox <gi...@apache.org>.
lidavidm commented on a change in pull request #12054:
URL: https://github.com/apache/arrow/pull/12054#discussion_r776817289
##########
File path: python/pyarrow/_compute.pyx
##########
@@ -1331,6 +1331,32 @@ class Utf8NormalizeOptions(_Utf8NormalizeOptions):
self._set_options(form)
+cdef class _RandomOptions(FunctionOptions):
+ def _set_options(self, length, initializer):
+ if initializer == 'system':
+ self.wrapped.reset(new CRandomOptions(
+ CRandomOptions.FromSystemRandom(length)))
+ return
+
+ if not isinstance(initializer, int):
+ try:
+ initializer = hash(initializer)
+ except TypeError:
+ raise TypeError(
+ f"initializer should be 'system', an integer, "
+ f"or a hashable object; got {initializer!r}")
Review comment:
Is it useful to `raise from` here to preserve the original context?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org