Algorithm step execution improvements

[nrsyed]

@waxlamp @as6520 @cfunk1210

As @waxlamp stated in a comment on a recent PR:

One note: the structure that old tinker engine used for algorithmic design is rather dangerous--it uses "stringly typed" algorithm phase names that are dispatched from the main execute() method. This strikes me as needlessly complex and unpythonic, so perhaps we can file an issue to replace that with a more robust, more typesafe, and more pythonic mechanism. (We would need some buy-in from @as6520 on this; I am operating on a memory of him saying he's on board with this in general.)

Currently, the step name (Initialize, FeatureExtraction, WorldDetection, NoveltyClassification, NoveltyAdaption, NoveltyCharacterization) is passed to the execute() method of the algorithm (or algorithm adapter) in the form of a string (e.g., algorithm.execute("FeatureExtraction", *args). The execute() method then calls the appropriate algorithm method based on the string.

Instead of relying on execute() as an intermediary, is there any reason the relevant methods couldn't be called directly (e.g., algorithm.feature_extraction(*args)?

[waxlamp]

Instead of relying on execute() as an intermediary, is there any reason the relevant methods couldn't be called directly (e.g., algorithm.feature_extraction(*args)?

The short answer is "no, there's nothing stopping us". The longer answer is, in order to do this properly, we need fuller support of smqtk. But since @Pikus16 is working on that, I think that barrier (if it even is a barrier) should be removed soon.

[as6520]

The main motivation for adding an adapter was to provide an abstraction for adding glue code since the algorithms have similar but not necessarily the same API. Here is an example

class Algorithm1:
      def __init__(self):
            pass

       def world_detection(self, feature_dict, logit_dict):
            # Doing something with the 2 dicts

 class Algorithm2:
      def __init__(self):
            pass

       def world_detection(self, logit_dict):
            # Doing something with the logit dicts     

The input requirements of these algorithms are slightly different, thus they can't be called directly by a protocol without adding conditional statements to account for the difference in input requirements. To address these, the adapter was added to make a call to functions of the algorithms with the appropriate input. If I recall correctly, the motivation for adding execute was to provide a general call with a string to describe the semantics of a step along with the state that is used to satisfy the requirements of the algorithm. That way the protocol didn't have to account for minor differences between algorithms, that work was delegated to the adapter which was written by someone who was familiar with the input-output of the algorithms.

I am not sure how smqtk handles this, although I don't have a lot of experience with it.

[nrsyed]

The call to execute could still be replaced by appropriately named methods in the adapter (instead of using a string), correct?

[waxlamp]

The call to execute could still be replaced by appropriately named methods in the adapter (instead of using a string), correct?

Exactly. This is the pythonic way to approach this problem, and we should use it. The idea is to design algorithm adapters appropriate to each protocol or use case, then arrange for any implementing algorithms to conform to the adapter's API.

There are cases where that may not be possible, such as where someone has already written a lot of code with disparate APIs that nonetheless need to be brought together. However, even in that scenario, I think you'd still want to design a uniform API to bring all those pieces together, rather than immediately fall back to stringly-typed method execution dispatch.

[as6520]

I am good with replacing the execute method with a named method.