-
Notifications
You must be signed in to change notification settings - Fork 115
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
Extension of the TornadoExecutionPlan API to allow backend and device filters from the API #357
Extension of the TornadoExecutionPlan API to allow backend and device filters from the API #357
Conversation
… the execution plan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just have a look at the files that were changed to make sure that the date in the license header is updated to 2024. E.g. in the TornadoBackend.java
, TornadoExecutionPlan.java
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, some minor comments:
- Regarding the update of the year in the copyright headers for the files that have been altered.
- Java doc description for the new methods. Especially to clarify the use of predicate for the methods that use the
TornadoBackend
and theTornadoDevice
as predicates.
@@ -17,11 +17,14 @@ | |||
*/ | |||
package uk.ac.manchester.tornado.api; | |||
|
|||
import java.util.List; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the year in the copyright header
@@ -31,6 +34,10 @@ public interface TornadoDriver { | |||
|
|||
TornadoDevice getDevice(int index); | |||
|
|||
List<TornadoDevice> getAllDevices(); | |||
|
|||
List<TornadoDevice> getDevicesWithPredicate(Predicate<? super TornadoDevice> predicate); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a java doc for this method. What is the context of the predicate in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed this method. We don't need it in this interface.
tornado-api/src/main/java/uk/ac/manchester/tornado/api/TornadoDeviceMap.java
Outdated
Show resolved
Hide resolved
private final List<TornadoBackend> backends; | ||
|
||
public TornadoDeviceMap() { | ||
numBackends = coreRuntime.getNumDrivers(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we refactor the name of that method to coreRuntime.getNumBackends
? Or does it break consistency within the runtime class? If it breaks consistency we can do it in a later PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. I think we can apply this refactor now.
tornado-api/src/main/java/uk/ac/manchester/tornado/api/TornadoDeviceMap.java
Outdated
Show resolved
Hide resolved
@@ -20,7 +20,7 @@ | |||
|
|||
import org.junit.Before; | |||
|
|||
import uk.ac.manchester.tornado.api.TornadoDriver; | |||
import uk.ac.manchester.tornado.api.TornadoBackend; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the year in copyright header.
@@ -22,14 +22,14 @@ | |||
|
|||
import uk.ac.manchester.tornado.api.ImmutableTaskGraph; | |||
import uk.ac.manchester.tornado.api.TaskGraph; | |||
import uk.ac.manchester.tornado.api.TornadoDriver; | |||
import uk.ac.manchester.tornado.api.TornadoBackend; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the year in copyright header.
@@ -18,22 +18,23 @@ | |||
|
|||
package uk.ac.manchester.tornado.unittests.loops; | |||
|
|||
import static org.junit.Assert.assertEquals; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the year in copyright header.
@@ -26,12 +26,12 @@ | |||
|
|||
import uk.ac.manchester.tornado.api.ImmutableTaskGraph; | |||
import uk.ac.manchester.tornado.api.TaskGraph; | |||
import uk.ac.manchester.tornado.api.TornadoDriver; | |||
import uk.ac.manchester.tornado.api.TornadoBackend; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the year in copyright header.
@@ -18,27 +18,28 @@ | |||
|
|||
package uk.ac.manchester.tornado.unittests.virtualization; | |||
|
|||
import static org.junit.Assert.assertEquals; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the year in copyright header.
…DeviceMap.java Co-authored-by: Thanos Stratikopoulos <34061419+stratika@users.noreply.github.com>
…uery' into feat/api/device-query
…DeviceMap.java Co-authored-by: Thanos Stratikopoulos <34061419+stratika@users.noreply.github.com>
All changes done. |
PR description updated with the latest API version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jjfumero, LGTM.
Improvements ~~~~~~~~~~~~~~~~~~ - `beehive-lab#344 <https://github.com/beehive-lab/TornadoVM/pull/344>`_: Support for Multi-threaded Execution Plans. - `beehive-lab#347 <https://github.com/beehive-lab/TornadoVM/pull/347>`_: Enhanced examples. - `beehive-lab#350 <https://github.com/beehive-lab/TornadoVM/pull/350>`_: Obtain internal memory segment for the Tornado Native Arrays without the object header. - `beehive-lab#357 <https://github.com/beehive-lab/TornadoVM/pull/357>`_: API extensions to query and apply filters to backends and devices from the ``TornadoExecutionPlan``. - `beehive-lab#359 <https://github.com/beehive-lab/TornadoVM/pull/359>`_: Support Factory Methods for FFI-based array collections to be used/composed in TornadoVM Task-Graphs. Compatibility ~~~~~~~~~~~~~~~~~~ - `beehive-lab#351 <https://github.com/beehive-lab/TornadoVM/pull/351>`_: Compatibility of TornadoVM Native Arrays with the Java Vector API. - `beehive-lab#352 <https://github.com/beehive-lab/TornadoVM/pull/352>`_: Refactor memory limit to take into account primitive types and wrappers. - `beehive-lab#354 <https://github.com/beehive-lab/TornadoVM/pull/354>`_: Add DFT-sample benchmark in FP32. - `beehive-lab#356 <https://github.com/beehive-lab/TornadoVM/pull/356>`_: Initial support for Windows 11 using Visual Studio Development tools. - `beehive-lab#361 <https://github.com/beehive-lab/TornadoVM/pull/361>`_: Compatibility with the SPIR-V toolkit v0.0.4. - `beehive-lab#366 <https://github.com/beehive-lab/TornadoVM/pull/363>`_: Level Zero JNI Dependency updated to 0.1.3. Bug Fixes ~~~~~~~~~~~~~~~~~~ - `beehive-lab#346 <https://github.com/beehive-lab/TornadoVM/pull/346>`_: Computation of local-work group sizes for the Level Zero/SPIR-V backend fixed. - `beehive-lab#360 <https://github.com/beehive-lab/TornadoVM/pull/358>`_: Fix native tests to check the JIT compiler for each backend. - `beehive-lab#355 <https://github.com/beehive-lab/TornadoVM/pull/355>`_: Fix custom exceptions when a driver/device is not found.
Description
This PR adds an extension to the
TornadoExecutionPlan
API in TornadoVM to allow developers to query backends and devices by applying filters. This API is designed as a consequence of the inconsistencies when obtaining a device using different APIs (more details in the next Section).This API fixes this inconsistencies by giving the user a new Data Structure, called
TornadoDeviceMap
in which developers can query all backends, devices and apply filters.Examples of use:
We can apply more complex queries as follows:
As part of this API, we needed to refactor some internal classes. The concept of Driver is not the same as we have in TornadoVM compared to the OS driver. Thus, all old driver classes became the Backend classes.
Problem description
The main problem is that to query the number of devices, developers need to access through the
TornadoRuntime.getRuntime
class. However, to select a device or a backend, developers need to the ExecutionPlan.This PR fixes this inconsistencies by providing a more concise and expressive API to filter devices and backends without the need to instantiate or interact with the TornadoVM Core Runtime (internal class).
Backend/s tested
Mark the backends affected by this PR.
OS tested
Mark the OS where this PR is tested.
Did you check on FPGAs?
If it is applicable, check your changes on FPGAs.
How to test the new patch?