Small fixes to `COPY_DATA`

ecmwf-ifs / field_api

Apache License 2.0

3 stars 8 forks source link

Small fixes to `COPY_DATA` #22

Closed awnawab closed 8 months ago

awnawab commented 8 months ago

This PR applies two small fixes to COPY_DATA:

The use of the OpenACC runtime function ACC_DEVICEPTR in COPY_DIM_CONTIGUOUS is replaced with acc host_data to align it with COPY_2D
The case of the fully contiguous field is promoted to the top of the nested SELECT CASE. This is the best possible scenario for data transfers and we should check this first rather than at the end.

The last point resulted in a more than 10% gain in data transfer speeds for CLOUDSC

FussyDuck commented 8 months ago

All committers have signed the CLA.

pmarguinaud commented 8 months ago

So what you mean is that we are spending a lot of time in the nested SELECT CASE constructs ? If so, we should probably take some more radical action, like storing a pointer to the copy function in the object itself.

See for instance : https://github.com/pmarguinaud/field_api/pull/new/naan-copy-data-fix

awnawab commented 8 months ago

Yes I think there was definitely an overhead for the nested SELECT CASE constructs. I really like your solution with determining the copy method at field construction time and storing a pointer to it, thanks so much for suggesting it 😄 Could I please merge that commit to this PR and test it?

awnawab commented 8 months ago

Hi @pmarguinaud,

I tested your COPY_FUNC fix and it indeed restores all the lost performance, thanks again for that! Could I please merge your naan-copy-data-fix branch to this PR? I need another small fix on top of that for transferring very large arrays. After that we can close this PR.

awnawab commented 8 months ago

Hi @pmarguinaud. Thanks again for contributing the fix. Could I please merge your contribution to this PR (and add another small commit on top) so we can close this soon?

pmarguinaud commented 8 months ago

Hello Ahmad,

Yes please merge my branch in your PR.

Regards