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

add whole-slide tiled read/write demos for measuring GPUDirect Storage (GDS) I/O performance #452

Merged
merged 3 commits into from
Nov 30, 2022

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Nov 18, 2022

These demos are intended to accompany a blog post describing tiled read and write operations of whole slide images.

change folder name

add back missing function

update paths searched for resize.tiff

add description of how to obtain test data
@grlee77 grlee77 added doc Improvements or additions to documentation non-breaking Introduces a non-breaking change labels Nov 18, 2022
@grlee77 grlee77 requested a review from a team as a code owner November 18, 2022 21:40
Copy link
Contributor

@gigony gigony left a comment

Choose a reason for hiding this comment

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

Thanks Greg! Looks good to me!

examples/python/gds_whole_slide/README.md Outdated Show resolved Hide resolved
Comment on lines 143 to 154
# def get_config(self):
# # cc_config = self.compressor.get_config() if self.compressor else None
# return {
# "id": self.codec_id,
# "compressor_config": None, # cc_config,
# }

# @classmethod
# def from_config(cls, config):
# # cc_config = config.get("compressor_config", None)
# # compressor = get_codec(cc_config) if cc_config else None
# return cls() # compressor=compressor)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be removed? or want to keep this for other purpose?

grlee77 and others added 2 commits November 23, 2022 12:48
Co-authored-by: Gigon Bae <gigony@gmail.com>
@grlee77
Copy link
Contributor Author

grlee77 commented Nov 30, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 8b3df72 into rapidsai:branch-22.12 Nov 30, 2022
Comment on lines +69 to +75
if dtype == 'uint8':
img = image_gpu
assert img.dtype == cp.uint8
else:
img = image_gpu.astype(dtype)
else:
raise NotImplementedError("only testing for uint8 and uint16")
Copy link
Member

@jakirkham jakirkham Jan 20, 2023

Choose a reason for hiding this comment

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

This double else looks odd. What was intended?

Looking at the error message would have thought this

Suggested change
if dtype == 'uint8':
img = image_gpu
assert img.dtype == cp.uint8
else:
img = image_gpu.astype(dtype)
else:
raise NotImplementedError("only testing for uint8 and uint16")
if dtype == 'uint8':
img = image_gpu
assert img.dtype == cp.uint8
elif dtype == 'uint16':
img = image_gpu
assert img.dtype == cp.uint16
else:
raise NotImplementedError("only testing for uint8 and uint16")

Though maybe the error is simply not needed? However then the if is probably unnecessary too

Suggested change
if dtype == 'uint8':
img = image_gpu
assert img.dtype == cp.uint8
else:
img = image_gpu.astype(dtype)
else:
raise NotImplementedError("only testing for uint8 and uint16")
img = image_gpu.astype(dtype)

Copy link
Member

Choose a reason for hiding this comment

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

cc @bdice

Copy link
Contributor

Choose a reason for hiding this comment

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

For context, I was trying to update #474 which reinstates some style checks that were disabled during the GitHub Actions transition. The Python code fails to parse because it has two else: blocks.

Copy link
Member

Choose a reason for hiding this comment

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

Thoughts @grlee77?

Copy link
Member

Choose a reason for hiding this comment

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

Creating issue ( #484 ) to track

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Improvements or additions to documentation non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants