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

ENH: Add a few bitarray functions and tests #4525

Merged
merged 10 commits into from
Apr 26, 2024

Conversation

matthewturk
Copy link
Member

In prep for some other work, here are additional items for bitarrays that make them nicer to use. Tests are added, and I think I got the edge cases.

@matthewturk
Copy link
Member Author

xor is probably not necessary to implement, but it could be.

@matthewturk
Copy link
Member Author

If this passes, could I get a review on it?

@neutrinoceros
Copy link
Member

@yt-fido test this please

@matthewturk
Copy link
Member Author

Hmm, this doesn't look like my error, but maybe it is? Did I miss marking something as C++?

+ python tests/nose_runner.py
Traceback (most recent call last):
  File "/home/fido/workspace/yt_py310_git/tests/nose_runner.py", line 9, in <module>
    from yt.config import ytcfg
  File "/home/fido/workspace/yt_py310_git/yt/__init__.py", line 14, in <module>
    from yt.data_objects.api import (
  File "/home/fido/workspace/yt_py310_git/yt/data_objects/api.py", line 1, in <module>
    from . import construction_data_containers as __cdc, selection_objects as __sdc
  File "/home/fido/workspace/yt_py310_git/yt/data_objects/construction_data_containers.py", line 17, in <module>
    from yt.data_objects.selection_objects.data_selection_objects import (
  File "/home/fido/workspace/yt_py310_git/yt/data_objects/selection_objects/__init__.py", line 1, in <module>
    from .boolean_operations import (
  File "/home/fido/workspace/yt_py310_git/yt/data_objects/selection_objects/boolean_operations.py", line 5, in <module>
    from yt.data_objects.selection_objects.data_selection_objects import (
  File "/home/fido/workspace/yt_py310_git/yt/data_objects/selection_objects/data_selection_objects.py", line 31, in <module>
    from yt.utilities.lib.marching_cubes import march_cubes_grid, march_cubes_grid_flux
ImportError: /home/fido/workspace/yt_py310_git/yt/utilities/lib/marching_cubes.cpython-310-x86_64-linux-gnu.so: undefined symbol: _Z13eval_gradientPiPdS0_S0_

@chrishavlin
Copy link
Contributor

Hmm, this doesn't look like my error, but maybe it is? Did I miss marking something as C++?

@matthewturk not your error, that's the flaky build error that happens on the full testsuite frequently. I'm actually not sure why it happens...

@chrishavlin
Copy link
Contributor

@yt-fido test this please

@neutrinoceros
Copy link
Member

I'm not sure either, though I suspect a symptom of race condition in the parallel build (possibly related: #4611). It's been happening since we switched jenkins from Python 3.8 to 3.10

@@ -14,7 +14,21 @@ cimport numpy as np

cdef inline void ba_set_value(np.uint8_t *buf, np.uint64_t ind,
np.uint8_t val) noexcept nogil:
# This assumes 8 bit buffer
# This assumes 8 bit buffer. If value is greater than zero (thus allowing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanna say I very much appreciate this detailed comment.

@@ -26,6 +26,7 @@ def test_inout_bitarray():
b = ba.bitarray(arr=arr_in)
arr_out = b.as_bool_array()
assert_equal(arr_in, arr_out)
assert_equal(b.count(), arr_in.sum())

# Let's check we can do something interesting.
arr_in1 = np.random.random(32**3) > 0.5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you haven't touched these lines, but it looks like another spot where we missed replacing np.random.random with the newer syntax (see #4733). if you want to update this whole function while you're here you'd want to put a

    rng = np.random.default_rng()

up above then replace all the np.random.random calls here with rng.random. We can also do this in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the change -- thanks!

@matthewturk matthewturk added this to the 4.4.0 milestone Apr 26, 2024
@matthewturk
Copy link
Member Author

self-merging because the tests pass and I want to get some stuff done before Chris leaves for the day (and this will make that easier)

@matthewturk matthewturk merged commit 28a963e into yt-project:main Apr 26, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants