Skip to content

FEAT: function to pad array borders #2682

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

Merged
merged 3 commits into from
Jan 9, 2020
Merged

Conversation

9prady9
Copy link
Member

@9prady9 9prady9 commented Dec 3, 2019

Addresses #2676 partially - allows the user to pad inputs and pass to convolve.

Performance comparison of indexing based padding vs kernel in this PR for a HD image is posted
#2676 (comment)

@sebastienleclaire
Copy link

If possible, it would be nice to support periodic padding as I always manually pad periodic image before convolve. It It is slow and memory consuming. I think my application would benefit from this kind of ability.

@9prady9
Copy link
Member Author

9prady9 commented Dec 9, 2019

@sebastienleclaire That shouldn't be hard I think. I wouldn't be able to say much about performance unless we run nvprof on it.

@sebastienleclaire
Copy link

For information, this is the function I use for periodic (circularly) padding (it could be use to write and debug new code):

array paddingArrayCircularly(const array& arrayToPad,
bool isPaddingDim0, bool isPaddingDim1, bool isPaddingDim2, bool isPaddingDim3)
{
int dim0 = arrayToPad.dims(0);
int dim1 = arrayToPad.dims(1);
int dim2 = arrayToPad.dims(2);
int dim3 = arrayToPad.dims(3);

	array id0 = isPaddingDim0 ? join(0, seq(dim0 - 1, dim0 - 1), seq(0, dim0 - 1), seq(0, 0)) : seq(0, dim0 - 1);
	array id1 = isPaddingDim1 ? join(0, seq(dim1 - 1, dim1 - 1), seq(0, dim1 - 1), seq(0, 0)) : seq(0, dim1 - 1);
	array id2 = isPaddingDim2 ? join(0, seq(dim2 - 1, dim2 - 1), seq(0, dim2 - 1), seq(0, 0)) : seq(0, dim2 - 1);
	array id3 = isPaddingDim3 ? join(0, seq(dim3 - 1, dim3 - 1), seq(0, dim3 - 1), seq(0, 0)) : seq(0, dim3 - 1);

	return  arrayToPad(id0, id1, id2, id3);
}

I'm pretty sure that whatever you come up with will be faster and will consume less memory. As it stand right now, This function use approximately twice the memory inside the array "arrayToPad" to work.

@9prady9
Copy link
Member Author

9prady9 commented Dec 10, 2019

@sebastienleclaire For the same sizes as you tried, periodic padding has the following global read/write efficiency.

    Kernel: void cuda::padBorders<float, af_border_type=3>(cuda::Param<float>, cuda::CParam<cuda::Param>, int, int, int, int, unsigned int, unsigned int)
         25                            gld_efficiency             Global Memory Load Efficiency      79.64%      79.64%      79.64%
         25                            gst_efficiency            Global Memory Store Efficiency      88.75%      88.75%      88.75%

@sebastienleclaire
Copy link

I am not sure what the above mean (gld_efficiency and gst_efficiency), but thank you for the periodic padding!

@9prady9
Copy link
Member Author

9prady9 commented Dec 10, 2019

That is global memory read and write efficiency metric names.

Add periodic padding support for pad fn
syurkevi
syurkevi previously approved these changes Jan 2, 2020
@syurkevi syurkevi added this to the v3.7.0 milestone Jan 2, 2020
@syurkevi
Copy link
Contributor

syurkevi commented Jan 2, 2020

👍 👍
Need this for a client project. Adding 3.7 milestone.

@umar456 umar456 merged commit 1a11988 into arrayfire:master Jan 9, 2020
@9prady9 9prady9 deleted the pad_borders branch January 10, 2020 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants