Gleethos/neureka

Internal Backend Code Refactor

Opened this issue · 1 comments

Gleethos commented

The Problem
The Algorithm layer of the backend has turned into a bit of a mess
with respect to some operations.
More specifically there are Algorithm
implementations which actually perform tasks that exceed their responsibility
in the sense that some of them deal with the Function AST instead of
focusing merely on a simple set of tensors as input.
The responsibility of breaking down the AST into something that is actually
executable lies at least one layer above it, the Operation layer to say the least.
At least from an architectural point of view it makes sense to shift some complexity
up the hierarchy as there is currently lots of room in the Operation sub-types.

To be more specific however, the goal here is to refactor away the usage of
AbstractDeviceAlgorithm::executeFor(...) and AbstractDeviceAlgorithm::executeDeviceAlgorithm(..)
as well as remove the need for the ExecutionPreparation interface by explicitly embedding execution call
preparations into every algorithm implementation instead of relying on this unnecessarily complicated
abstraction... This is partially due to the DRY principle being taken too seriously.

Which leads to the second ambitious goal of this issue,
which is the removal of the AbstractFunDeviceAlgorithm, whose existence
is hard to justify in light of the additional backend API surface and complexity / ambiguity
it brings (A regular Algorithm implementation is more than enough).

Expected Result
A simpler backend implementation, which is more readable, maintainable, extendable
and serves as a nice example for future operations/algorithms and device specific implementations!

Risks
Strictly and technically speaking, this cleanup will most likely lead to breaking changes
in the public API of the library, however, these changes will not affect anything outside the main
backend package, which is an internal part of the library.
This is explicitly specified as part of the documentation which is why these changes will be
part of a minor release, even if it is technically possible that a careless client
may experience a breaking upgrade.
(This is extremely unlikely, as these internal classes barely have a use outside the context of the library internals)