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 PaddedDiskArray and pad #219

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

add PaddedDiskArray and pad #219

wants to merge 6 commits into from

Conversation

rafaqz
Copy link
Collaborator

@rafaqz rafaqz commented Jan 27, 2025

Ive had this use case a few times that you want to pad a disk array for windowing or fixing chunk alignment, and you don't want to make a copy of the file.

This PR adds a PaddedDiskArray wrapper and a DiskArrays.pad function that pads any disk array by custom amounts above and below the original array on each axis. The fill value for the padded area can be set with the fill keyword, which is zero by default.

Chunk offsets are updated to match the padding

@rafaqz rafaqz requested a review from meggart January 27, 2025 19:10
Copy link
Collaborator

@meggart meggart left a comment

Choose a reason for hiding this comment

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

Thanks for starting this, this would be useful indeed.

src/pad.jl Outdated
end

readblock!(A::PaddedDiskArray, data, I...) = _readblock_padded!(A, data, I...)
readblock!(A::PaddedDiskArray, data, I::AbstractVector...) =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious why this is necessary, are the fallbacks too slow here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure I understood this question, I think we need readblock! to fill the padding zeros/etc? Or should I do that another way?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment was about the type signature of the arguments. I would suggest to do define readblock!(A::PaddedDiskArray, data, I::AbstractUnitRange{<:Int}...) and let DiskArrays figure out complicated cases like unsorted vectors.

@meggart
Copy link
Collaborator

meggart commented Feb 12, 2025

@rafaqz do you need support for fixing some of the chunk issues?

@rafaqz
Copy link
Collaborator Author

rafaqz commented Feb 12, 2025

Should be ok, just haven't had time to get back to it yet

@rafaqz
Copy link
Collaborator Author

rafaqz commented Feb 19, 2025

@meggart I think this is good to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants