Skip to content

Conversation

franzpoeschel
Copy link
Contributor

@franzpoeschel franzpoeschel commented May 17, 2024

We currently don't support memory selections yet, e.g. when you have a 2D memory buffer but want to write only an arbitrary block from it. That block is then non-contiguous, but ADIOS2 has so-called memory selections to support this.

We currently don't have an API that would be able to expose this (except in Python where the native Python buffer protocol is powerful enough to express this; we currently throw an error when detecting this strides in selection are inefficient, not implemented!). Since the loadChunk()/storeChunk() API is somewhat convoluted by now anyway, I didn't want to add even further overloads.

Instead, this PR proposes a new syntax for setting up load/store calls:

RecordComponent E_y;

E_y.prepareLoadStore()
    .offset({0, 5}) // optional
    .extent({1,5}) // optional
    .enqueueLoad<int>(); // -> std::shared_ptr<int>

E_y.prepareLoadStore()
    .offset({0, 5}) // optional
    .extent({1,5}) // optional
    .enqueueLoadVariant(); // ->std::variant<std::shared_ptr<char>, ...>

E_y.prepareLoadStore()
    .offset({0, 5}) // optional
    .extent({1,5}) // optional
    .enqueueStore<int>(); // -> DynamicMemoryView<int>, i.e. Span API

std::shared_ptr<int> data;
E_y.prepareLoadStore()
    .offset({0, 5}) // optional
    .extent({1,5}) // optional
    .withSharedPtr(data) // or withUniquePtr(), withRawPtr(), withContiguousContainer()
    .memorySelection({{1,1},{5,5}}) // call only available after having called withSharedPtr()
    .enqueueStore(); // -> void, no template parameter needed since withSharedPtr(data) already gives the type

E_y.prepareLoadStore()
    .withSharedPtr(data) // or withUniquePtr(), withRawPtr(), withContiguousContainer()
    .enqueueLoad(); // load the whole dataset into the buffer

This API makes it easier to add functionality without going through every single overload and adapting it. Further, it makes the API more consistent, e.g. offset and extent are now always optional and not just for those calls that implement a default logic.

With this PR, the old API calls are now fully implemented via the new API, giving it a good test coverage.

TODO:

  • What about Python? Maybe stick with the buffer protocol there for now.
  • Experiment: Return futures instead of bare return types: Use the chance to introduce a safe async API. Introduce option .noop_future() to disable future return types.

@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch 2 times, most recently from 727cbbc to ddcdfc9 Compare May 22, 2024 11:44
@ax3l ax3l self-requested a review May 31, 2024 17:50
@ax3l ax3l self-assigned this May 31, 2024
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from c05d535 to ba7c582 Compare June 7, 2024 12:41
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from ba7c582 to d7eb891 Compare June 24, 2024 12:47
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch 2 times, most recently from 332932f to efb0876 Compare June 25, 2024 13:56
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from efb0876 to 39e3d71 Compare June 26, 2024 11:48
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch 4 times, most recently from 47d497b to b2bc355 Compare July 22, 2024 10:21
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from b699520 to 3430b6d Compare July 23, 2024 14:05
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from 9b5acff to bd7b378 Compare September 3, 2024 13:06
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from 208c381 to e51e635 Compare November 15, 2024 14:45
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch 2 times, most recently from c8be94b to 8e455b4 Compare December 17, 2024 11:09
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from 8e455b4 to 430fe3b Compare February 21, 2025 12:06
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from 430fe3b to e98c840 Compare March 26, 2025 14:33
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch 2 times, most recently from 190674f to e04f76d Compare April 4, 2025 08:32
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from b99bb95 to d05e029 Compare April 22, 2025 09:09
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch 2 times, most recently from 54eb6e8 to 0d3e248 Compare August 7, 2025 14:17
RecordComponent::loadChunkAllocate_impl(internal::LoadStoreConfig cfg)
{
using T = std::remove_extent_t<T_with_extent>;
// static_assert(!std::is_same_v<T, std::string>, "EVIL");

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
std::static_pointer_cast<T>(std::move(ptr)),
std::move(offset),
std::move(extent));
// static_assert(!std::is_same_v<T_with_extent, std::string>, "EVIL");

Check notice

Code scanning / CodeQL

Commented-out code Note

This comment appears to contain commented-out code.
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from 0d3e248 to 66caf5f Compare August 8, 2025 12:23
@franzpoeschel franzpoeschel force-pushed the adios2-memory-selection branch from 66caf5f to 3e8f374 Compare August 11, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants