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

Performance improvements for fixed resolution buffer #2507

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions glue/core/fixed_resolution_buffer.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@
dimension, otherwise an error will be raised.
"""

print('compute_fixed_resolution_buffer', data.uuid, bounds, target_data.uuid if target_data is not None else None, target_cid.label if target_cid is not None else None, subset_state, broadcast, cache_id)

astrofrog marked this conversation as resolved.
Show resolved Hide resolved
if target_data is None:
target_data = data

Expand All @@ -143,24 +145,41 @@
if cache_id is not None:

if subset_state is None:
# Use uuid for component ID since otherwise component IDs don't return
# Use uuid for data and component ID since otherwise component IDs don't return
# False when comparing two different CIDs (instead they return a subset state).
# For bounds we use a special wrapper that can identify wildcards.
current_array_hash = (data, bounds, target_data, target_cid.uuid, broadcast)
current_array_hash = (data.uuid, bounds, target_data.uuid, target_cid.uuid, broadcast)
else:
current_array_hash = (data, bounds, target_data, subset_state, broadcast)
current_array_hash = (data.uuid, bounds, target_data.uuid, subset_state, broadcast)

Check warning on line 153 in glue/core/fixed_resolution_buffer.py

View check run for this annotation

Codecov / codecov/patch

glue/core/fixed_resolution_buffer.py#L153

Added line #L153 was not covered by tests

current_pixel_hash = (data, target_data)
current_pixel_hash = (data.uuid, target_data.uuid)

if cache_id in ARRAY_CACHE:
if ARRAY_CACHE[cache_id]['hash'] == current_array_hash:
return ARRAY_CACHE[cache_id]['array']

# To save time later, if the pixel cache doesn't match at the level of the
# data and target_data, we just reset the cache.
if cache_id in PIXEL_CACHE:
if PIXEL_CACHE[cache_id]['hash'] != current_pixel_hash:
PIXEL_CACHE.pop(cache_id)
# For pixel transformations, we don't necessarily need the cache_id to match, as coordinate
# transformations can be the same for different callers - for instance, layer artists for
# subsets might need the same transformations over all subsets and the same as for the actual
# data. Therefore, instead of extracting the cache_id entry from PIXEL_CACHE, we instead loop
# over PIXEL_CACHE, and for each pixel component ID, we find the first entry that matches the
# hash and the bounds, since that should uniquely identify the transformation. Note that we
# still use cache_id because we still want to make sure we save results from different layers
# to the cache in case any layers get removed or changed.
matching_pixel_cache = {}
for chid in PIXEL_CACHE:
if PIXEL_CACHE[chid]['hash'] == current_pixel_hash:
for ipix, pix in enumerate(data.pixel_component_ids):
if (
ipix not in matching_pixel_cache and
ipix in PIXEL_CACHE[chid] and
PIXEL_CACHE[chid][ipix]['bounds'] == bounds
):
matching_pixel_cache[ipix] = PIXEL_CACHE[chid][ipix]

Check warning on line 178 in glue/core/fixed_resolution_buffer.py

View check run for this annotation

Codecov / codecov/patch

glue/core/fixed_resolution_buffer.py#L178

Added line #L178 was not covered by tests

else:

matching_pixel_cache = None

# Start off by generating arrays of coordinates in the original dataset
pixel_coords = [np.linspace(*bound) if isinstance(bound, tuple) else bound for bound in bounds]
Expand All @@ -179,17 +198,11 @@

for ipix, pix in enumerate(data.pixel_component_ids):

# At this point, if cache_id is in PIXEL_CACHE, we know that data and
# target_data match so we just check the bounds. Note that the bounds
# include the AnyScalar wildcard for any dimensions that don't impact
# the pixel coordinates here. We do this so that we don't have to
# recompute the pixel coordinates when e.g. slicing through cubes.

if cache_id in PIXEL_CACHE and ipix in PIXEL_CACHE[cache_id] and PIXEL_CACHE[cache_id][ipix]['bounds'] == bounds:
if matching_pixel_cache is not None and ipix in matching_pixel_cache:

translated_coord = PIXEL_CACHE[cache_id][ipix]['translated_coord']
dimensions = PIXEL_CACHE[cache_id][ipix]['dimensions']
invalid = PIXEL_CACHE[cache_id][ipix]['invalid']
translated_coord = matching_pixel_cache[ipix]['translated_coord']
dimensions = matching_pixel_cache[ipix]['dimensions']
invalid = matching_pixel_cache[ipix]['invalid']

Check warning on line 205 in glue/core/fixed_resolution_buffer.py

View check run for this annotation

Codecov / codecov/patch

glue/core/fixed_resolution_buffer.py#L203-L205

Added lines #L203 - L205 were not covered by tests

else:

Expand Down