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

[RFC|oneMKL]Enable programmatic versioning #561

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

vmalia
Copy link

@vmalia vmalia commented Aug 5, 2024

Motivation

Inspiration

  • SYCL versioning does not add the revision number:

image

  • OpenMP specification itself keeps only MAJOR.MINOR version, with ongoing progress marked as Technical Report <number>. The _OPENMP macro provides a release date which maps onto a specification version, like this:
#define _OPENMP 202111    //202111: OpenMP 5.2

Proposal

  • <MAJOR><MINOR><REVISION> such that MINOR and REVISION have two digits.
  • Propose a macro for oneAPI and suggest a naming scheme for component macro names.

@rscohn2
Copy link
Member

rscohn2 commented Aug 6, 2024

@uxlfoundation/oneapi-spec-maintainers: Please review this versioning proposal

doc/versioning.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@andrewtbarker andrewtbarker left a comment

Choose a reason for hiding this comment

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

I do not understand what is being proposed here.

Is the idea that each implementation of the spec gets to decide how to provide a version to the user (whether by a version string, a #define, an int getSpecVersion() function)? Then this change to the spec only specifies that the end of this string/macro/int has a certain representation?

doc/versioning.rst Outdated Show resolved Hide resolved
doc/versioning.rst Outdated Show resolved Hide resolved
doc/versioning.rst Outdated Show resolved Hide resolved
@vmalia vmalia changed the title [RFC][oneMKL]Enable programmatic versioning [RFC]Enable programmatic versioning Aug 20, 2024
doc/versioning.rst Outdated Show resolved Hide resolved
@vmalia
Copy link
Author

vmalia commented Aug 20, 2024

Is the idea that each implementation of the spec gets to decide how to provide a version to the user (whether by a version string, a #define, an int getSpecVersion() function)? Then this change to the spec only specifies that the end of this string/macro/int has a certain representation?

Yes. The goal of this RFC is to ensure that every implementation of the spec provides a numerically comparable value for its compliance with the spec version, through the proposed macros.

The implementation can additionally provide more ways to query that information, like APIs, but we do not mandate that in this proposal.

doc/versioning.rst Outdated Show resolved Hide resolved
@vmalia
Copy link
Author

vmalia commented Sep 4, 2024

@akukanov @mkrainiuk @rscohn2 @spencerpatty if everything looks okay, then please provide your approval. Thank you!

@vmalia vmalia changed the title [RFC]Enable programmatic versioning [RFC|oneMKL]Enable programmatic versioning Sep 4, 2024
Copy link
Contributor

@spencerpatty spencerpatty left a comment

Choose a reason for hiding this comment

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

Acceptable from my perspective. Approved

@vmalia
Copy link
Author

vmalia commented Sep 17, 2024

@aelizaro @paveldyakov @lhuot @zettai-reido @sknepper can you please provide your reviews? Thank you!


The macros for each domain are listed as follows:

| ONEMKL_BLAS_SPEC_VERSION
Copy link
Contributor

Choose a reason for hiding this comment

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

If/When we switch to oneMATH name for the Specification and Interfaces project, will we also switch the macro names ? maybe we want to delay this merge until those are made to not have to deprecate/switch ... ?

Copy link
Author

@vmalia vmalia Sep 17, 2024

Choose a reason for hiding this comment

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

@spencerpatty The changes you mentioned are from oneapi-src/oneMKL#564 which is an RFC in one of the implementations of this spec. The implementation can choose to take any direction, whether or not it is in compliance with the specification.

@Rbiessy you approved this PR earlier so there are 2 questions -

  1. Does your RFC([RFC] oneMKL Interface renaming oneapi-src/oneMKL#564) block this RFC?
  2. Does your RFC aim to make changes to the oneAPI specification eventually? If yes, can you please create a placeholder Issue/PR in this project and mention it here? If not, please note that your oneMKL Interfaces RFC breaks compliance with the oneAPI specification as of the current status of your RFC.

Note point number 5 in https://github.com/mkrainiuk/oneMKL/blob/rfcs/README.md#rfc-ratification-process

Copy link
Author

Choose a reason for hiding this comment

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

I had an offline discussion with the team and we came to an agreement that the oneMATH proposal has a lot of work ahead of it, and that name change will have to be made in not just oneMKL Interfaces, but also in multiple places across oneAPI spec and other documents. Since this change will have a wider impact than just this specific part of document where this PR targets, it is better to make that change at that time. It will not affect the core version schema or approach proposed here.
This PR and RFC is NOT blocked by @Rbiessy 's RFC in oneMKL Interfaces.

Copy link
Contributor

Choose a reason for hiding this comment

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

It does not matter to me which RFC is implemented first. The RFC for renaming is planning to rename the occurrences of all the macros using ONEMKL.
The RFC describes how it impacts the specification. I read the ratification process, it does not say that an issue needs to be created. @mkrainiuk could this be clarified in the RFC process? I created #580 but I have some concerns the discussions will be split in 2 different places now.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @Rbiessy, good point, we need to clarify it in the ratification process, and I think any changes related to API should be discussed in Spec only, since opensource is just an implementation of the Spec, I believe it should just follow what was decided for Spec. In this particular case I guess the problem is that we have opensource project migration (which is not related to the Spec) combined with renaming (which is Spec related and should be started in the Spec).

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for clarifying, @mkrainiuk . @spencerpatty @Rbiessy Let's proceed with this RFC with the oneMKL naming. When the oneMATH proposal is made to the oneAPI spec, it can address these macros and other changes across the oneAPI spec simultaneously.

+++++++++++++++++++++++++++++++++++++++++++++

This is the oneMKL specification which is part of the oneAPI specification version 1.0.0.
Each oneMKL domain must define a preprocessor macro to represent the version of the specification that the implementation is compliant with.
Copy link

@al42and al42and Sep 20, 2024

Choose a reason for hiding this comment

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

Do all headers provide this info? E.g.,

  • Will #include "oneapi/mkl.hpp" provide ONEMKL_*_SPEC_VERSION for all domains?
    • If a domain is not supported/installed, will it be undefined or 0?
  • Will #include "oneapi/mkl/rng.hpp" provide ONEMKL_RNG_SPEC_VERSION?
  • Will #include "oneapi/mkl/rng.hpp" provide ONEMKL_BLAS_SPEC_VERSION?
  • Will #include "oneapi/mkl/rng/device.hpp" provide ONEMKL_RNG_SPEC_VERSION?

Copy link
Author

@vmalia vmalia Sep 24, 2024

Choose a reason for hiding this comment

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

@al42and good question. The specification(and this RFC) does not define any implementation-specific header files for oneMKL, so the location of the macro can be decided by the implementation itself.

If a domain is not supported/installed, will it be undefined or 0?

For an undefined domain implementation, this, https://oneapi-src.github.io/oneMKL/create_new_backend.html#create-wrappers shows how the wrappers in oneMKL Interfaces work and how the implementation can handle errors for unimplemented backends. Note that this is different than the oneAPI specification that we have here. More clarification on that: https://github.com/oneapi-src/oneMKL/blob/develop/README.md#faqs

I added a rule for the macro numbering for the non-compliance of an implementation. Thanks!


.. code-block:: c

// For oneAPI 1.2 rev 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Can domain be aligned with provisional spec version?
E.g. the latest available release is 1.4. provisional https://github.com/uxlfoundation/oneAPI-spec/releases

If yes, how we should update the macro in this case?

Copy link
Author

Choose a reason for hiding this comment

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

@paveldyakov the provisional versioning (https://github.com/uxlfoundation/oneAPI-spec/blob/main/doc/versioning.rst#provisional-versions) is for specifications that are pending approval.

It is highly unlikely that we will have implementations that will comply with provisional specifications unless the approval of the spec happens later than the implementation in the backend library. If the backend is ready before the spec, the macro can point to a future non-provisional revision because the spec release follows a predictable convention.

From the above link:

A specification may be published before approval, but must be labeled provisional. Provisional specifications are published as a series of revisions until approval. After approval, the provisional label is removed and the rev is set to 1.

| ONEMKL_STAT_SPEC_VERSION
| ONEMKL_VM_SPEC_VERSION

The specification version can be created by appending all digits of the specification version in the format of <MAJOR><MINOR>. MINOR version always uses two digits. This version can be used to check the compatibility of the implementation with the specification version. Note that the revision is not included here because it reflects changes only for the specification document without affecting the implementation. If the implementation is not compliant with any release of the specification, then the macro must have a numerical value of `000`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we link to a document that describes how/when the MAJOR, MINOR, and revision versions get updated?

Copy link
Author

Choose a reason for hiding this comment

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

Added at the end of this section.

+++++++++++++++++++++++++++++++++++++++++++++

This is the oneMKL specification which is part of the oneAPI specification version 1.0.0.
Each oneMKL domain must define a preprocessor macro to represent the version of the specification that the implementation is compliant with.
Copy link
Contributor

Choose a reason for hiding this comment

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

The oneMKL spec allows for an implementation of the spec to implement just 1 API and throw unsupported for all of the other APIs in that domain. (See https://github.com/uxlfoundation/oneAPI-spec/blob/main/source/elements/oneMKL/source/architecture/api_design.inc.rst)
In that case, is the implementation considered conformant to whatever version of the spec has the APIs that are included in the implementation (albeit mostly unsupported), or is that implementation considered non-compliant?

Copy link
Author

@vmalia vmalia Sep 26, 2024

Choose a reason for hiding this comment

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

My understanding is that the implementation will still be compliant, because it follows the definitions of the specification to handle errors for unimplemented APIs.

However, the implementation would not be conformant to the standards that the industry would expect "oneMKL" backend libraries to be, because of lack of most APIs.
Alternatively, it could be a feature that a specific hardware-software use case only requires or supports certain APIs as per their app's needs.

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.

10 participants