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

Repo Naming #22

Closed
FranklandJack opened this issue Nov 9, 2022 · 12 comments
Closed

Repo Naming #22

FranklandJack opened this issue Nov 9, 2022 · 12 comments
Assignees
Milestone

Comments

@FranklandJack
Copy link
Contributor

There appears to be an empty repo for the Unified Runtime spec: https://github.com/oneapi-src/unified-runtime-spec

Should we:
a. Move the headers/spec from here to that repo? (we will loose all the pull requests and issues)
b. Delete https://github.com/oneapi-src/unified-runtime-spec and rename this repo to unified-runtime-spec
c. Delete https://github.com/oneapi-src/unified-runtime-spec and do not rename this repo.

@pbalcer
Copy link
Contributor

pbalcer commented Nov 22, 2022

In the case of level zero, the level-zero repo contains the loader and a copy of the headers, whereas the level-zero-spec repo contains the YAML spec, scripts, and pre-generated content.

So let's be consistent and go with either option a or b. The latter is probably the least work.

While we are discussing repo structure, should we have an additional unified-runtime-adapters repo to move all the implementations and the shared code to?

@FranklandJack
Copy link
Contributor Author

I'm wondering if we need to have separate repos? We've already discussed adding testing to this repo and the less dependencies a Unified Runtime user needs to pull in to make use of the project, the easier it'll be. I'm not particularly tied to this, but it might be worth considering putting the loaders, adapters, testing, spec/headers and utilities all in the same place for ease of use?

If we did want to look at deleting or renaming any repos I think we would need additional permissions to what we currently have.

@pbalcer
Copy link
Contributor

pbalcer commented Nov 22, 2022

That also seems like a reasonable solution. It would simplify syncing between the spec and the loader. But we'd have to rethink the structure of the repo.

@FranklandJack
Copy link
Contributor Author

True, but I don't think restructuring would be too disruptive at this early stage, especially if we improve the build system of this project to expose dependencies to downstream users in a more firewalled fashion, we should then have more flexibility to move things around.

Currently all the PI adapters live in the intel LLVM fork within the SYCL subdirectory, which is where PI is defined, so I guess it would be analogues to adding them here?

@zackwaters
Copy link
Contributor

Regarding Level Zero, it makes sense to have the spec separate from the implementation since there can be multiple implementations of Level Zero across devices. The loader and headers were separate in an effort to allow the spec to move forward freely while loaders/headers can be snapped at certain key milestones.

I don't know that we have these issues with Unified Runtime. If not then I would vote for option c which is to delete https://github.com/oneapi-src/unified-runtime-spec and not rename this repo.

@pbalcer
Copy link
Contributor

pbalcer commented Nov 30, 2022

@FranklandJack I think we can follow the repo structure similar to level-zero, where implementations live in their own directory (https://github.com/oneapi-src/level-zero/tree/master/source/drivers). I'm hoping that we will create a PR with these changes (and the loader + null implementation) soon. Unless there are different ideas?

@FranklandJack
Copy link
Contributor Author

Sure something like that sounds reasonable to me.

We also need to discuss adding the test suite and any shared utilities (e.g. the command buffer software implementation when we eventually do that) somewhere, something like this perhaps?:

.
├── source
│   ├── adapters
│   │   ├── cuda
│   │   ├── hip
│   │   ├── null implementation
│   │   ├── level zero
│   │   └── opencl
│   ├── loader
│   └── utils
├── spec <- current content of this repo.
└── test

@kbenzie
Copy link
Contributor

kbenzie commented Nov 30, 2022

This mostly looks sensible to me, although I don't think the current state of the repo should be wholesale moved into the spec subdirectory.

I'm not fond of the spec source living in scripts/core*, so I think that should be moved to spec/core*.

I think scripts should stay were it is and perhaps merged with the loader scripts?

The existing content in source could probably move to test.

@pbalcer
Copy link
Contributor

pbalcer commented Nov 30, 2022

Yes - definitely, we are sorely missing tests :-) For the time being, the utils directory is going to be empty because work is going to happen in SYCL, but we can definitely do something like that.

As for where the spec is going to live, I was thinking more of just leaving it where it is (scripts/core). I like @kbenzie's suggestion to move just the spec's YAML files to a separate directory.

There's quite some overlap between the existing scripts and the loader scripts, so it makes sense to have them be located in the same directory.

@FranklandJack
Copy link
Contributor Author

Sure, this sounds very reasonable to me! Since this is beyond the scope of this ticket (which is now to close the delete the spec repo) I've created #76 to actually do the restructuring, so maybe we should continue the discussion on a directory layout there?

@FranklandJack
Copy link
Contributor Author

Following discussions within the Unified Runtime working group it has been decided to go with option c: Delete https://github.com/oneapi-src/unified-runtime-spec and do not rename this repo..

@kbenzie kbenzie self-assigned this Dec 1, 2022
@kbenzie
Copy link
Contributor

kbenzie commented Feb 21, 2023

https://github.com/oneapi-src/unified-runtime-spec has now been removed.

@kbenzie kbenzie closed this as completed Feb 21, 2023
@kbenzie kbenzie added this to the 0.7 milestone Aug 3, 2023
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

4 participants