Implementation and Example for EVM code are very complicated and almost not to be understood

[siebenkopf]

I have some doubts on the implementation of the EVM code, which are the following:

1. Most importantly, the return types are neither standard nor intuitive:

   vast/vast/opensetAlgos/EVM.py

   Line 293 in 2ad7fa2

   yield (

   
   a) Instead of turning the function into a generator, you should rather return the values in a useful data type (see next point).
   b) Instead of returning a (nested) 2-element tuple, I would rather return a (nested) dictionary with the first element of the tuple as key, and the second as value.
   c) When returning from the inference function, the first element is always identical "probs":

   vast/vast/opensetAlgos/EVM.py

   Line 340 in 2ad7fa2

   yield ("probs", (batch_to_process, probs))

   
   There is no reason to provide this as a result, it will be ignored in any function that calls this one.

I do understand why you are returning it this way -- to be able to call it in parallel processes -- but handling dictionaries in the calling function is at least as simple.

2. Providing input parameters in an argument parser structure is very uncommon:

   vast/vast/opensetAlgos/EVM.py

   Line 133 in 2ad7fa2

   args,

   
   Any function that wants to use your functions will need to either take your provided arg parser, or replicate your parser structure. The more common way is to have a separate parameter with a proper default value. Then, you can collect your parameters in a dictionary and pass them to the function, such as:

params = dict(tailsize=0.1, distance_metric="euclidean")
results = EVM_Training(..., **params)

3. As far as I understood your code, the chunk size is based on the number samples, where we could have many per class. However, the concatenation of features is tested for equality with the chunk size:

   vast/vast/opensetAlgos/EVM.py

   Line 209 in 2ad7fa2

   if len(temp) == args.chunk_size:

   
   It might happen that I this test is never met, when we have more samples in the collection than we have defined as chunk size.

4. For that the Example.iynb should serve as an example on how to use your functions, there are many parts that are very difficult to be understood: https://github.com/Vastlab/vast/blob/2ad7fa23cb8f0f1acb71d6b6d9aced6a70562e0c/vast/opensetAlgos/Example.ipynb
   a) Cell 2 uses some pre-defined data that is downloaded from your web page, but it is explained nowhere what this data contains.
   b) The line 2 in cell 3 contains a nested list, map, list, lambda, apply function that is impossible to follow, especially when we do not know how the data looks like.
   c) Cell 10 processes the complicated (see my point 1 above) output of your EVM_Training function using a combination of dict, list, zip, *, [1] which does not help in any way to understand how to handle the results of the training.
   d) Cell 11 contains a cat, list, dict, list, zip, *, [1], values combination. WTF. I have no idea what this is doing.

I would highly recommend to change the implementation to be more user-friendly and provide an example that show-cases how to use your functions with simple python, i.e., without any of: cat, zip, *, [1], values, list, dict, especially not in combination in one line (as far as possible).

[akshay-raj-dhamija]

Thanks for your valuable feedback Manuel!
I understand this code can be a bit overwhelming for anyone trying to understand some design decisions. Most decisions have been made keeping performance on large datasets into consideration and till a certain extent can be understood by understanding how these functions are called here.

1. The reason the function returns an iterator is simple we do not want to hold EVM models in memory, so as models are created they are returned and the calling process can write them to disk. This becomes especially important when running grid search because the number of models increases drastically.
2. While I agree with you, this is a design choice I had made and it isn't unheard of e.g. it was used heavily in the earlier versions of fast.ai ... The helper version EVM_Params was provided to make this process easier. While I do understand the pain I believe it might be too late to change this without breaking things across the board.
3. As the comment explains the chunk size is currently based on the number of classes. According to this assumption, the code is doing exactly what's intended ... do you disagree?
4. I agree the example is horribly written, but since I don't have much bandwidth would appreciate any contributions.

[siebenkopf]

Let me try to explain how things might be easier accessible for someone who is not working with huge models.

1. I can understand the reason for point 1a). Having a generator function is able to solve these issues. However, the return types should still be (nested) dictionaries and not (nested) two-element tuples. Especially for the inference function, this gets tricky. Here is some code that I need to write in order to run an evaluation for a single vector of features and a single model:

probabilities = next(vast.opensetAlgos.EVM_Inference([0], {0:features}, self, gpu_id, model))[1][1][0].tolist()

As you might see, there are a lot of indexes, and it took me at least 10 trials to get the indexes right.
Note that I have stored the parameters in the calling class, so I can just use self as the arguments.
Ideally, I would have wanted to write:

probabilities = vast.opensetAlgos.EVM_Inference(features, model, gpu_id, **parameters)

2. I am just viewing this from the perspective of someone who wants to use your code. As you mentioned yourself, it was used in early versions of fast.ai, but then they realized that this is not a good style.

3. For the chunk_size this is fine. I just was not able to follow your code, so I thought that you were concatenating features from negatives (which might end up in concatenating several features at a time and, therewith, increasing the counter by more than one at a time), but apparently this does not seem to be the case.

4. My problem is that I do not understand the original code in all details, so it is hard for me to provide better examples.