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

Simplifying da.map_overlap's boundary padding #4252

Open
jakirkham opened this issue Nov 27, 2018 · 4 comments
Open

Simplifying da.map_overlap's boundary padding #4252

jakirkham opened this issue Nov 27, 2018 · 4 comments
Labels

Comments

@jakirkham
Copy link
Member

Currently da.map_overlap supports edge paddings of different kinds along different dimensions. This complicates things like using da.pad within da.map_overlap.

AFAIK there are no use cases for different edge paddings on different dimensions. The closest thing that I know of is to pad some dimensions and not others (i.e. having different depths per dimension), which can still be cast in terms of having the same padding for all dimensions (just some are trivial). Given this, I'd like to propose deprecating boundary arguments that are anything other than simple values (e.g. padding mode or constant for all dimensions).

@jakirkham jakirkham changed the title Simplifying da.map_overlap's overlaps Simplifying da.map_overlap's boundary padding Nov 27, 2018
@mrocklin
Copy link
Member

mrocklin commented Nov 27, 2018 via email

@jakirkham
Copy link
Member Author

Good point. Though these would still be possible by applying da.pad repeatedly with the intended padding before calling da.map_overlap.

Should add that my general inclination is actually to push all users looking for explicit padding from Dask to use da.pad instead of relying on da.map_overlap to support this behavior directly. Often functions (at least in the image processing domain) do some implicit padding internally anyways.

@jakirkham
Copy link
Member Author

jakirkham commented Jun 20, 2019

Thoughts on handling this in 2.0?

xref: #4973

@mrocklin
Copy link
Member

I think I prefer to have a convenience keyword argument in map_overlap. I'd be more than happy if that called pad. I don't care strongly about different edge paddings on different dimensions.

In terms of handling this before 2.0 I don't have strong thoughts. To me this doesn't seem to be a large priority to me personally (I haven't heard anyone complain) and so I don't think that it should block release.

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

No branches or pull requests

2 participants