yt-project / yt

Main yt repository
http://yt-project.org
Other
454 stars 272 forks source link

Deprecation warning in GDF (and maybe more?) #4918

Closed chrishavlin closed 1 month ago

chrishavlin commented 1 month ago

While checking out the status of running answer tests with pytest, and trying to run yt/frontends/gdf/tests/test_outputs.py, it failed due to a deprecation warning.

To reproduce, run the following script in developer mode (python -X dev gdf_test.py)

# gdf_test.py
import yt  
ds = yt.load('sedov/sedov_tst_0004.h5')
_ = ds.index

and it raises a deprecation warning:

/Users/chavlin/src/yt_/yt_dev/yt/yt/frontends/gdf/data_structures.py:118: DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)

This originates from

https://github.com/yt-project/yt/blob/341cb00c150a58d407e6d6cc66e308b77296bfc9/yt/frontends/gdf/data_structures.py#L115-L123

self.grid_levels is defined as np.zeros((self.num_grids, 1), "int32"), so that self.grid_levels[gi] ends up being an array with shape (1,) which is coerced to an int within get_box_grids_level.

I suspect that you'd get the same warning when get_box_grids_level is called in athena and amrex frontends but haven't checked yet. One potential fix is to just change the grid_levels definition to np.zeros((self.num_grids, ), "int32") in the base _initialize_grid_arrays, but I'm not sure if anything relies on the shape being (self.num_grids, 1)

neutrinoceros commented 1 month ago

Nice catch ! I didn't know about -X dev, looks very useful ! I think you're right that changing the signature of get_box_grids_level would fix it, but I think it would also make the code less clear, so I'd prefer to fix the call sites instead if we can. Something like

diff --git a/yt/frontends/gdf/data_structures.py b/yt/frontends/gdf/data_structures.py
index 1bb7a88b6..0817c2aa1 100644
--- a/yt/frontends/gdf/data_structures.py
+++ b/yt/frontends/gdf/data_structures.py
@@ -115,7 +115,7 @@ class GDFHierarchy(GridIndex):
             get_box_grids_level(
                 self.grid_left_edge[gi, :],
                 self.grid_right_edge[gi, :],
-                self.grid_levels[gi],
+                self.grid_levels[gi].item(),
                 self.grid_left_edge,
                 self.grid_right_edge,
                 self.grid_levels,
neutrinoceros commented 1 month ago

Ah, trying to fix callsites I found that this was already partially done in https://github.com/yt-project/yt/pull/4422 Mostly likely, the difference is that some call sites, as the one you found, are not covered by tests.

chrishavlin commented 1 month ago

I didn't know about -X dev, looks very useful ! Ya, I didn't know about it either! but was looking for an easy way to trigger the deprecation warning for sake of reproducing it since this test isn't covered by a pytest test.