Repository navigation
Improve the performance of tiled decompression - #14430
Merged
Merged
Conversation
Contributor
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
Contributor
|
👋 Thank you for your draft pull request! Do you know that you can use |
Member
|
We should try and re-write _iter_array_tiles to not use Numpy as I think this slows things down a bit |
astrofrog
reviewed
Feb 22, 2023
Cadair
commented
Feb 22, 2023
astrofrog
force-pushed
the
performance_issues_decompress_tile
branch
from
February 23, 2023 14:39
0ef5093 to
e9f8f07
Compare
Cadair
force-pushed
the
performance_issues_decompress_tile
branch
from
February 23, 2023 16:40
6d15639 to
b2921ce
Compare
astrofrog
reviewed
Feb 23, 2023
astrofrog
reviewed
Feb 23, 2023
|
|
||
| # If more than half the data is requested, read in all the heap. | ||
| # TODO: decide what heuristic we actually want here | ||
| if np.product(buffer_shape) > 0.5 * np.product(data_shape): |
4 of 8 tasks
astrofrog
added
Affects-dev
astrofrog
force-pushed
the
performance_issues_decompress_tile
branch
from
March 6, 2023 11:37
b2921ce to
fff7370
Compare
* Read the whole heap in one go if a significant fraction of the data is needed. * Minimize FITS_rec access and cast to np.array as soon as possible * Consolidate code to read in tiles/heap * Other small optimizations
astrofrog
force-pushed
the
performance_issues_decompress_tile
branch
from
March 7, 2023 09:43
0825234 to
55c3580
Compare
Cadair
commented
Mar 7, 2023
Comment on lines
+143
to
+145
| if algorithm in ("RICE_1", "RICE_ONE", "PLIO_1") and tile_size < len( | ||
| tile_buffer | ||
| ): |
…ng .data might be more appropriate than using .section
astrofrog
marked this pull request as ready for review
March 7, 2023 14:15
Member
|
@saimn - I believe this is ready for review - this speeds things up by a factor of a few in our new tiled compression algorithm, to bring it a lot closer to the original performance in previous astropy versions. |
saimn
reviewed
Mar 7, 2023
| # We have to calculate the tilesize from the shape of the tile not the | ||
| # header, so that it's correct for edge tiles etc. | ||
| settings["tilesize"] = prod(actual_tile_shape) | ||
| elif compression_type in ("RICE_1", "RICE_ONE"): |
Member
|
@saimn - I've addressed your comments 😄 |
saimn
approved these changes
Mar 8, 2023
saimn
left a comment
saimn
left a comment
Contributor
There was a problem hiding this comment.
Nice improvements, thanks @Cadair & @astrofrog !
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a work in progress for the moment as we continue to iterate on the performance hot spots.