Code readability and potential confusion in tcc/visualize_alignment.py

[awmillerUCSD]

Hello,
a section of the tcc code in visualize_alignment.py has high potential for confusion and misuse. The align function is defined as follows:

def align(candidate_feats, query_feats, use_dtw):
  """Align videos based on nearest neighbor in embedding space."""
  if use_dtw:
    _, _, _, path = dtw(candidate_feats, query_feats, dist=dist_fn)
    _, uix = np.unique(path[0], return_index=True)
    nns = path[1][uix]
  else:
    nns = []
    for i in range(len(candidate_feats)):
      nn_frame_id, _ = get_nn(query_feats, candidate_feats[i])
      nns.append(nn_frame_id)
  return nns

The function call is:
nns.append(align(embs[query], embs[candidate], use_dtw))

The positional arguments for the query and candidate features are reversed. Clearly, we do not want to iterate over the candidate frame matching it to the reference. There is no logical error as the arguments are passed in to the function in reverse order but it may lead to issues downstream if these functions are built upon.

The function definition should read:

def align(query_feats,candidate_feats, use_dtw):
  """Align videos based on nearest neighbor in embedding space."""
  if use_dtw:
    _, _, _, path = dtw(query_feats,candidate_feats, dist=dist_fn)
    _, uix = np.unique(path[0], return_index=True)
    nns = path[1][uix]
  else:
    nns = []
    for i in range(len(query_feats)):
      nn_frame_id, _ = get_nn(query_feats[i], candidate_feats)
      nns.append(nn_frame_id)
  return nns

[debidatta]

Yes, you are right. Thanks for the suggestion. This commit changes the order of query and candidate.