jackgeraghty/aqp

AQP is a modular pipeline built to enable the comparison and testing of different quality metric configurations.

aqp's Issues

Abstract Node Base Class

Currently, the Node class operates like an abstract class, but has no actual restrictions imposed on instantiating a Node object (as an abstract class should).

Refactor to a "proper" abstract class.

Expanded Pipeline Diagram

After refactoring the code to move away from NetworkX, the functionality to create an expanded version of the pipeline diagram hasn't fully worked. This issue could have existed before since it was only noticed when creating a complicated pipeline for testing out three visqol configurations against each other.

Issue arises from trying to connect nodes together that belong to either a LoopNode or an EncapsulationNode. To create the expanded diagram, the Loop or Encapsulation Nodes need to be connected to the first node in their subgraph. Then the leaf nodes of that subgraph need to be connected to the children of the Loop or Encapsulation Nodes. On the top level, this works ok. It is only when you have nested Encapsulation Nodes within LoopNodes and then another Loop Node within the Encapsulation Node. In this case, an inner LoopNode, belonging to the EncapsulationNode, is a leaf node and the previous setup tries to connect this node to the nodes belonging to the outermost LoopNode.

It has been improved from how broken it was before. Now, when finding leaf nodes, it ignores LoopNodes. The output from this is a lot cleaner than it was before. The issue still remains though of it not connecting the expanded version properly.

Refactor Output Keys

Each node currently requires an output_key, this key can be set to None but in the instance where it isn't it holds a string value. This forces a node to have at most one output_key. From implementing some of the ViSQOL nodes there is a need for multiple values to be added to the results dictionary within a single node.

A potential refactor of this functionality would include changing the output key to be either a string OR a list of strings. When implementing a node an order can then be defined for the output keys and consequently ordered as such in a JSON definition..

Could look like for example:

{
    "node": {
        "output_keys": ["key_one", "key_two", ...]
}

Parallel Execution of Nodes

The pipeline, while having branching configurations etc. still operates in a sequential order. The ability to run branches in parallel would speed up the execution of the pipeline. This would be a large architecture change and would require major refactoring of different nodes and functions to accommodate it.

ViSQOL Giving Incorrect Results

Running ViSQOL on a the GenSpeech dataset and graphing the predicted MOS versus the sample MOS highlighted that somewhere along the pipeline when calculating either the MOS, fvnsims, vnsims etc. something isn't working as intended. The graph produced has a straight, vertical line for the sampleMOS vs predicted MOS. Predicted MOS is giving the same output for all files.

This is a tough issue to track exactly where the calculations are going wrong. Likely this is happening after the spectrograms are created and in the degraded patch alignment/patch similarity functions.

https://github.com/JackGeraghty/AQP/blob/main/nodes/visqol_nodes/patchalignmentnode.py
https://github.com/JackGeraghty/AQP/blob/main/nodes/visqol_nodes/patchsimilaritynode.py
https://github.com/JackGeraghty/AQP/blob/main/nodes/visqol_nodes/mosmappernode.py
https://github.com/JackGeraghty/AQP/blob/main/nodes/visqol_nodes/referencepatchnode.py
https://github.com/JackGeraghty/AQP/blob/main/qualitymetrics/visqol/dsp.py

Cycle Checks and Unreachable Nodes

There is no checking for cycles in the current version of the pipeline. The pipeline operates on the hope that the user doesn't try and create cycles in the DAG. It is possible to check for cycles so this check should be introduced.

It is possible to create Nodes that are not connected to any other node. This is typically a mistake by the user and can lead to time debugging. This could be avoided by performing a check when creating the graph and indicating to the user that a node is not connected.

Unit Testing

Big issue, there's no unit testing and that's not good. Unit testing needs to be introduced for most of the pipeline, as well as some integration tests. This is likely to be a big version update.