Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

One view or two views? #96

Closed
marcverhagen opened this issue May 1, 2024 · 8 comments
Closed

One view or two views? #96

marcverhagen opened this issue May 1, 2024 · 8 comments

Comments

@marcverhagen
Copy link
Contributor

Because

The current SWT produces one view with timepoints using the basic labels and timeframes using binned labels. An earlier version had two views:

  • one with all timepoints with basic labels
  • one with with timeframes with binned labels and time points with binned labels, but only for those timepoints that are included in timeframes.

@owencking mentioned a while ago that he liked having the two views. Is that still the case?

One issue with two views is that the basic properties of the video are added to one of the views only:

 {
    "@type": "http://mmif.clams.ai/vocabulary/Annotation/v5",
    "properties": {
        "fps": 29.97,
        "frameCount": 3000.0,
        "duration": 100100.0,
        "durationTimeUnit": "milliseconds",
        "id": "a_1"
    }
}

That is done behind the screens either by the MMIF video helper or by some other MMIF code. I personally do not like that that annotation is only in one of the views.

Done when

No response

Additional context

No response

@owencking
Copy link
Collaborator

One reason I like two views is that it opens the possibility of separating them. For example, you might generate a MMIF file that includes both TimePoint and TimeFrame annotations, But, for downstream use, you might care only about the TimeFrame annotations. So you could drop the view with the TimePoint annotations before sending the data downstream.

Of course, that strategy works only if the annotations do not refer to each other. According to how TimeFrame annotations work in SWT, they are not interpretable without the target TimePoint annotations.

So, overall, here is what I think: If annotations refer to each other, then they should probably be in the same view. If annotations are referentially independent, then it is nice to have them in separate views. If they are in separate views, then I agree with Marc that it would be desirable for the properties element to appear in both.

@keighrim
Copy link
Member

keighrim commented May 1, 2024

That is done behind the screens either by the MMIF video helper or by some other MMIF code.

Document-level Annotation is added after clamsproject/mmif#134 but no one was using it so we added automatic "magic" helpers clamsproject/mmif-python#226.

I personally do not like that that annotation is only in one of the views.

I'm open to alternative suggestions, but if being only in one view is the problem, I don't see why we need to duplicate the document-level properties at every view.

@marcverhagen
Copy link
Contributor Author

I'm open to alternative suggestions, but if being only in one view is the problem, I don't see why we need to duplicate the document-level properties at every view.

It would be redundant of course to have it in both views, so from an information perspective it will never be a big deal to only having it in one view, beyond the minor nuisance to have to go look for it in multiple views. It is probably not worth messing with the magic helpers.

@marcverhagen
Copy link
Contributor Author

Implemented in 7bd76fd.

The views are now independent from each other. So the second view, the one with the timeframes and the postbin labels, has time points like this:

{
  "@type": "http://mmif.clams.ai/vocabulary/TimePoint/v4",
  "properties": {
    "timePoint": 1001,
    "label": "bars",
    "classification": {
      "bars": 1.0,
      "slate": 5.475348025993886e-16,
      "other_opening": 2.1893750462467355e-13,
      "chyron": 1.5494242178856287e-37,
      "credit": 7.562205466634266e-24,
      "other_text": 2.0932760879509115e-19
    },
    "id": "tp_1"
  }
}

These are referred to by time frames in the same view.

This time point however is pointing at the same time offset as a time point in the first view, which has a different identifier and a different classification because it uses the prebin labels. In the past we talked about perhaps using the targets property to have the timepoint above refer to the time point in the other view. I think I like using timePoint better.

@keighrim
Copy link
Member

keighrim commented May 6, 2024

I don't really see a point having the second set of TimePoint objects in the second view, except for simplifying the pointers in the TimeFrame::targets properties down from cross-view references. Beyond that very particular benefit, we will have issues like

  1. very bloated MMIF with redundant TP objects
  2. the second set is not actually full duplicates, but information-lost inferior copies, and will add confusion of interpreting the scores in the classification properties in the second (post-binned) TPs, as they are not clearly probabililty
  3. most importantly, losing a simple and straightforward way to re-use the TP annotations from the "raw" classification in a downstream app, without knowing the specific existence of the information-lost second set of TPs.

More details on the point 3.

With just one set of TPs, we can do something like this in a downstream app that only wants to deal with the TP annotations

def _annotate(self, mmif, params): 
    ...         
    tp_view = mmif.get_view_contains(AnnotationTypes.TimePoint)
    tps = list(tp_view.get_annotations(AnnotationTypes.TimePoint))
    ... # do stuff with timepoints, such as screen bug detection, moving/dynamic credit processing, etc. 

In the contrary, now the downstream developer has to know that getting the "latest" TP view will not give you the full picture. Hence need to do something like this to access the raw classification results ;

def _annotate(self, mmif, params): 
    ...         
    tp_view = mmif.get_view_contains(AnnotationTypes.TimePoint)  # so this view is actually contains the information-lost TPs
    app, version  = self._parse_TPview_producer_identifier
    x = 5.0  # or the swt version where TP duplication first introduced 
    y = ???  # if we decide not to do the duplication in the future
    if app == "swt-detection" and x <= version <= y:
       # need to re-iterate all views from the start to grab the "previous" view where the original raw classification is stored
        next view_id = tp_view.id
        correct_view_id = None
        for view in mmif.views:  # or for view in mmif.get_all_views_contain(AnnotationTypes.TimePoint)
            if view.id == next_view_id:
                tp_view = mmif[correct_view_id]
                break
            correct_view_id = view.id
    tps = list(tp_view.get_annotations(AnnotationTypes.TimePoint))
    ... # do stuff with timepoints, such as screen bug detection, moving/dynamic credit processing, etc. 

, which is not particularly elegant, and also requires close dependency to the implementation details in the specific versions of a specific app, and surely guaranteed to be prone to lots of bugs and high maintenance cost.

Given all that, going back to @owencking 's previous comment

If annotations refer to each other, then they should probably be in the same view.

I don't think they should, as seen in any multi-app pipeline where any annotation in a later view can refer back to annotations in any previous views without being in the same view. Even for a single-app pipeline (one app generates multiple views), the same mechanism still holds.

And finally, in the original issue description

time points with binned labels, but only for those timepoints that are included in timeframes.

What is the use case for the second set of TimePoint?

@marcverhagen
Copy link
Contributor Author

marcverhagen commented May 6, 2024

  1. very bloated MMIF with redundant TP objects
  2. the second set is not actually full duplicates, but information-lost inferior copies, and will add confusion of interpreting the scores in the classification properties in the second (post-binned) TPs, as they are not clearly probability
  3. most importantly, losing a simple and straightforward way to re-use the TP annotations from the "raw" classification in a downstream app, without knowing the specific existence of the information-lost second set of TPs.

None of these arguments are convincing to me: (1) the bloat is very minor, (2) the point of the second view is not to have duplicates but to add new information (the classification scores are probabilities, even though because of rounding they don't alway look it), (3) the raw classification can still be re-used (but see below).

However, this is a good argument and it should give us pause:

... now the downstream developer has to know that getting the "latest" TP view will not give you the full picture

I don't really see what the example code is trying to point out, but it is definitely a concern that picking out the right time points is a pain unless we add something in the metadata to help with that. We do have the labelset property and that should make selecting the right timepoints easier, but that may not be sufficient.

To me the issue is not so much whether there are one or two views, but rather whether it makes sense to have the extra time points with the postbin labels or not (this is also pointed out at the end of Keigh's comment). If it does add value to the user we should figure out the cleanest way to do that. This does not necessarily mean that SWT should include those extra time points, we could also have another utility that allows you to get the postbin labels easily.

Finally:

If annotations refer to each other, then they should probably be in the same view.

I don't think they should, as seen in any multi-app pipeline where any annotation in a later view can refer back to annotations in any previous views without being in the same view. Even for a single-app pipeline (one app generates multiple views), the same mechanism still holds.

MMIF indeed does not care, we have the machinary to go either way.

@marcverhagen
Copy link
Contributor Author

the classification scores are probabilities, even though because of rounding they don't alway look it

Not quite true, it would be if the NEG label were added

@marcverhagen
Copy link
Contributor Author

Following up on recent discussions in a GBH-Brandeis meeting we have the following observations:

  • There is not a very strong use case for having the extra set of time points with derivative labels, except for at experimentation time, in which case the standalone stitcher is probably a better option anyway.
  • The one view with timepoints with the full label set and timeframes with the postbin labels is just simpler to characterize.

So one view it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

3 participants