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

Figure out and enforce UR's "core" feature set #1267

Open
aarongreig opened this issue Jan 22, 2024 · 1 comment
Open

Figure out and enforce UR's "core" feature set #1267

aarongreig opened this issue Jan 22, 2024 · 1 comment

Comments

@aarongreig
Copy link
Contributor

During discussion of #537 it came up that we don't have a well defined notion of optionality in the UR spec. A specific example of this is that due to how they're tested, the vast majority of the object handle queries are de-facto optional. This was a deliberate decision, and it's practical because we've always relied heavily on exterior testing rather than the cts for acceptance criteria. Now that we're looking to improve CTS coverage so that we can rely on it for acceptance we should also consider the optionality question in terms of how features are tested and how it's represented in the spec. It was suggested during the discussion that we could analyse the SYCL CTS and e2e tests to get an idea of what our core feature set should look like, and given the e2e tests are what we're trying to match in terms of coverage in the CTS that seems like a sensible place to start.

@aarongreig
Copy link
Contributor Author

I think this comes down to "how big of a deal is it if an adapter doesn't implement x". Looking at sycl I've run into three answers to that question:

  • very big deal: no implementation == tests fail, sky falls, etc (non-optional)
  • big deal for some adapters: should return a correct value if called, but will never be called for some adapters ("unreachable" style optional)
  • big deal if unimplemented, no big deal if implementation isn't useful: hints and queries that can just return a default value/do nothing (usefulness optional)

For a time during this investigation I thought we might be able to do away with the UNSUPPORTED_ENUMERATION error code, but the existence of optionality mode #2 makes a compelling case: we have something which some adapters simply can't implement, and there isn't a harmless default they can do rather than implementing it. The API user (sycl) doesn't use these features when it knows the underlying adapter doesn't implement them, so having something loud (returning an error code) to do if anything does try to use them is sensible.

The outcome of this should be that we communicate which level of optional each feature is to API users and adapter implementers. This will need to be done primarily through the spec and the CTS, although we could make use of other vectors like warning messages once that mechanism matures. I have a plan on how to tackle this for queries:

CTS
Optionality mode #1 is the default way of testing stuff in the CTS, non success error code == fail. Mode #2 is already tested for queries, but by default rather than design. Currently all of the tested queries (not aware of any exceptions) are allowed to return UNSUPPORTED as alluded to in the OP on this issue. Ideally we should split the GetInfo tests into optional and non-optional, allowing us to choose which queries are allowed to return UNSUPPORTED.

We currently have at least some queries which are realistically optionality mode #3 that currently behave as if they're #2 in some adapters. We will need to go through and fix all of those as part of the testing refactor, each query should be scrutinized before we allow it to return UNSUPPORTED.

Spec
Queries that fall into modes #2 and #3 should be clearly marked as such in the spec. Mode #2 queries will just need to note that they might cause an UNSUPPORTED return if the adapter doesn't support them. Mode #3 queries should define minimums or defaults, the same way CL says like max image size queries return 0 if the device doesn't support images.

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

No branches or pull requests

1 participant