Skip to content

Conversation

lplewa
Copy link
Contributor

@lplewa lplewa commented Aug 22, 2024

No description provided.

@lplewa lplewa requested a review from a team as a code owner August 22, 2024 15:01
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we already have use cases for such API? Does OpenMP need it? I am just curious to understand how it is used in real life.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is restrict_storage//subnumspace one. In their example they want to allocate from every second target(a.k.a. every second memory). We can achieve this also with filters (which I'm going to implement next), but such basic API as create empty memspace and add or remove targets from it, is quite useful too in my opinion. i.e when you want to create pool which allocates from single specific numa node.

Currently we have "umfMemspaceCreateFromNumaArray" to cover this openMP case, but i will consider to remove it(or to not export it to be precise), when this and filtering patch is merged.

Copy link
Contributor

Choose a reason for hiding this comment

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

but i will consider to remove it

that is what I am worried about. The first release of UMF is coming soon. And we should care about backward compatibility.

So we should be careful with introducing new API - it is not regarding this particular PR, but rather a general rule we should follow. Also, we need to consider an option to mark some API as experimental - it will give our users a notion that experimental API is not stable and might be removed/changed in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

@vinser52 this would not be merged on v0.9.x branch (2025.0 oneAPI release)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but i will consider to remove it

that is what I am worried about. The first release of UMF is coming soon. And we should care about backward compatibility.

So we should be careful with introducing new API - it is not regarding this particular PR, but rather a general rule we should follow. Also, we need to consider an option to mark some API as experimental - it will give our users a notion that experimental API is not stable and might be removed/changed in the future.

As far i know we are releasing 0.9 because we do not want to commit to stable API (semver specyfication) There is few minor issues with our API they i did not have time to fix. Like wrong variable size, const, signed vs unsigned

@lplewa lplewa force-pushed the add-remove-new branch 2 times, most recently from b219cd2 to 5cf7c80 Compare September 13, 2024 12:06
@bratpiorka
Copy link
Contributor

@lplewa - is this ready for merge?

@lplewa lplewa merged commit 87f9cdc into oneapi-src:main Sep 17, 2024
70 checks passed
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.

4 participants