[SYCL][UR][HIP] s/ScopedContext/ScopedDevice

[ldrumm]

d1c92cb changed ScopedContext to take a device instead of a context thus sematically changing its meaning. This rename makes it clear what the intented usages are.

[hdelan]

Also would it be possible to do this in the CUDA adapter as well?

[ldrumm]

Also would it be possible to do this in the CUDA adapter as well?

I'll leave that for someone else, another time. This is a side effect of work on HIP only

[hdelan]

LGTM

[jchlanda]

I'm not sure if I see it in the same way, the fact that it now takes a ur_device_handle_t to construct the object is merely an implementation detail, the purpose is still the same, to manage the hipCtx_t and so the original name (ScopedContext) seems more fitting. I do not feel strong about it, so if the majority prefers the updated version I'm happy to roll with it.

[ldrumm]

I'm not sure if I see it in the same way, the fact that it now takes a ur_device_handle_t to construct the object is merely an implementation detail, the purpose is still the same, to manage the hipCtx_t and so the original name (ScopedContext) seems more fitting. I do not feel strong about it, so if the majority prefers the updated version I'm happy to roll with it.

Fair point. I did this because I got confused with the semantics, but yes - it is is still managing the context. I'll think about it a little, and then either close this PR, or convince you I'm right ;)

[hdelan]

I think from the viewpoint of the UR, it makes sense to think of this as a scoped UR device. It is an implementation detail of the plugins that we manage UR devices through native contexts. I think as long as we are using UR, where UR contexts mean something different to native contexts, we should try to make all user facing semantics involving contexts related to UR contexts, and not native contexts. Therefore IMO we should keep this as a (UR) device related feature.

[hdelan]

@jchlanda can we get this approved plz? @ldrumm can you maybe close and reopen the PR to retrigger the CI?

[jchlanda]

I understand where you are coming from and you make a good argument for it, perhaps we could fold in your reasoning in form of a comment, just in case anyone else wonders about the name?

@ldrumm did you decide if you'd like to keep this change?

I think from the viewpoint of the UR, it makes sense to think of this as a scoped UR device. It is an implementation detail of the plugins that we manage UR devices through native contexts. I think as long as we are using UR, where UR contexts mean something different to native contexts, we should try to make all user facing semantics involving contexts related to UR contexts, and not native contexts. Therefore IMO we should keep this as a (UR) device related feature.

[hdelan]

Thanks @jchlanda I think that sounds reasonable. @ldrumm would you be able to add a comment noting why the name change?

[jandres742]

no L0 changes but since L0 reviewers are requested, +1