Skip to content

automatic document-level Annotation recorder #226

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

Closed
keighrim opened this issue Jun 28, 2023 · 5 comments · Fixed by #227
Closed

automatic document-level Annotation recorder #226

keighrim opened this issue Jun 28, 2023 · 5 comments · Fixed by #227
Labels
✨N New feature or request

Comments

@keighrim
Copy link
Member

keighrim commented Jun 28, 2023

New Feature Summary

We have added a place to store document-level information by re-purposing Annotation at_type (clamsproject/mmif#134) but none of the apps have been taking advantage of the feature so far, AFIK. That said, I'd like to propose a way to automate some of the recording process implemented as a part of the SDK.

Implementation plan;

on Document side
  • Document class keeps a secondary properties2 attributes for temporary storage for document-level information.
  • Document.add_property() should add information to the properties2 to keep the original properties from the source document object from the top-level documents list intact.
automatic generation of Annotation objects
  • The annotation object should be generated from the properties2 attribute.
  • We can use the "sanitize" step (adding sanitized serialize #212) in the serialize() to generate those objects.
  • When a property from properties2 dict is "converted" to an Annotation annotation, it should be popped out to prevent duplicate recording.
    • (updated) ideally in a CLAMS app, serialization only happens when all the data processing has been done and all Annotations were added to the corresponding views. In such case, there's no "duplicate" recording/serialization hence this point is irrelevant. On the other hand, during development and testing, devs can use serialization for debugging purposes, so in that case, popping properties out from the temporary memory is not a good idea. So I decide not to pop out them.
  • So, which view to throw all these Annotation objects? I'm thinking whichever comes first during the serialization.
    • (updated) turned out, using the last view would be much easier implementation-wise and doesn't seem to hurt any functionality.
automatic loading of Annotation objects
  • Now we are talking about a third properties3 attributes to loading document properties from existing Annotation objects during de-serialization.
  • This should be read-only and shouldn't be serialized back into the Document instance when serializing.
  • As a result, we will have three properties dictionary, hence Document should provide get_property(prop_name) to look for all three dicts and return the found value.

Example

Suppose a slate detection app, having with code:

class SlateApp(ClamsApp):
   ...  # set attributes, __init__, etc

   def _annotate(self, mmif):
        for vd in mmif.get_documents_by_type(VideoDocument):
            v = mmif.new_view()
            self.sign_view(v)
            video = mmif_utils_videodocument.read(vd)
            for slate in self.detect_slates(video):
                v.new_annotation(TimeFrame, **slate.__dict__)

    def detect_slates(self, video: cv2.VideoCapture):
       ...  # code continues

Then in the mmif_utils_videodocument:

def read(vd: mmif.serialize.annotation.Document):
    v = cv2.VideoCapture(vd.location_path())
    vd.add_property('fps', v.get(CAP_PROP_FPS))  # this will save `fps=29.97` in the `properties2` dictionary
    return v

Then, you'll write something like this

in_mmif = json.loads("""{
  "metadata": {
    "mmif": "http://mmif.clams.ai/1.0.0"
  },
  "documents": [
    {
      "@type": "http://mmif.clams.ai/vocabulary/VideoDocument/v1",
      "properties": {
        "mime": "video",
        "id": "d1",
        "location": "file:///dogs.mp4"
      }
    }
  ],
  "views": []""")
app = SlateApp()    # note that in _annotate() method, there weren't a line to manually add `Annotation` annotations to the view
out_mmif = app._annotate(in_mmif)
out_json = out_mmif.serialize(pretty=true)

to see

{
  "metadata": {
    "mmif": "http://mmif.clams.ai/1.0.0"
  },
  "documents": [
    {
      "@type": "http://mmif.clams.ai/vocabulary/VideoDocument/v1",
      "properties": {
        "mime": "video",
        "id": "d1",
        "location": "file:///dogs.mp4"
      }
    }
  ],
  "views": [
    {
      "id": "v1",
      "metadata": {
        "timestamp": "2020-05-27T12:23:45",
        "app": "http://apps.clams.ai/slate-detector/versionX",
        "contains": {
          "http://vocab.lappsgrid.org/Annotation": {
            "document": "d1"
          },
          "http://vocab.lappsgrid.org/TimeFrame": {
            "document": "d1",
            "timeUnit": "frames"
          }
        }
      },
      "annotations": [
        {
          "@type": "http://vocab.lappsgrid.org/TimeFrame",
          "properties": {
            "document": "d1",
            "start": 400,
            "end": 2400,
            "id": "tf1"
          }
        },
        {
          "@type": "http://vocab.lappsgrid.org/Annotation",    # ta-da! This one is generated from `properties2` by the MMIF serializer code
          "properties": {
            "document": "d1",
            "id": "a1",
            "fps": "29.97"
          }
        }
      ]
    }
  ]
}

And finally, if there's a downstream processing:

downstream_input_mmif = Mmif(out_json)  # the same json from the above
vd = downstream_input_mmif.get_document_by_id('d1')
print(vd.properties)
print(vd.properties2)
print(vd.properties3)
print(vd.get_property('fps')) 

will show

{
  "mime": "video",
  "id": "d1",
  "location": "file:///dogs.mp4"
}   # original property staying intact
{  }   # nothing has added during current processing app
{
  "fps": "29.97"
}   # read from `Annotation` objects existed in the input MMIF (`out_json`),  completely volatile
"29.97"  # `get_property` should be smart enough to look for the key in all three dicts

Related

Alternatives

No response

Additional context

I propose this feature since I realized we must have all video-related MMIFs have Annotation for the FPS of the video, but none of the current video apps implements that. Without FPS information, we cannot perform evaluation of video apps solely based on the MMIF files without having access to the source video files because then time unit conversion is impossible without opening up the video files.

It's true that devs still need to update the existing apps to use this new feature (once it's ready) to have FPS properly "recorded", but having this feature will make future video apps development much easier, with combination with #221.

@kelleyl @MrSqually @snewman-aa I know you guys have been working on some video processing apps, and want to hear your feedback on this proposal.

@keighrim keighrim added the ✨N New feature or request label Jun 28, 2023
@clams-bot clams-bot added this to infra Jun 28, 2023
@github-project-automation github-project-automation bot moved this to Todo in infra Jun 28, 2023
@marcverhagen
Copy link
Contributor

I think I am missing something on what exactly the "recording process" is. I was thinking of this "capital A" annotation process as something that proceeds in the same way as regular view creation, that is, the app adds a view with he document-level annotation, and no extra machinery is needed.

@keighrim
Copy link
Member Author

You are correct regarding the current status of the Annotation situation (which is not being used at all in any existing apps I've seen). I'm just trying to provide some automated assistance to read and write Annotation objects for app developers.

The "recoding process" is the lines of code in the serialize() (or _serialize()) method of MMIF objects.

@snewman-aa
Copy link
Contributor

I've definitely been frustrated with how inelegant my handling of video document properties has been. I've been extracting the location, fps, etc and then passing those values around "loose" or making them app instance attributes which wouldn't work for handling multiple video documents.

I think this will make it easier to make apps more flexible.

It feels like something we'd want to have in the documents property, but I get that those are read-only after their initialized so I see why we're not using that.

I am pretty sure that having a "capital" Annotation annotation type will bring quite a bit of communicative confusion in the future

This was my first thought reading through this. vd.add_property() and .get_property are definitely intuitive though so I think this will be good for readability too. I just think seeing an Annotation type annotation of document properties in the MMIF might be a little confusing. We would just want to make it clear in the MMIF documentation I guess.

@keighrim
Copy link
Member Author

keighrim commented Jul 5, 2023

Adding Annotation objects to the last view can be problematic, for example when a NLP app generates many views for each previously existing TextDocument from OCR app, and adding doc-level Annotation for each document , all of them will be added to the last view that holds other annotations (NLP results) only about the last text documents.

@keighrim
Copy link
Member Author

4600e65 solves the problem raise in the above comment.

@github-project-automation github-project-automation bot moved this from Todo to Done in infra Jul 11, 2023
keighrim added a commit that referenced this issue Jul 24, 2023
* `view.metadata.contains.some_at_type` dictionary
* the basic idea is the same as proposed in #226
* mmif.utils.vdhelper.get_annotation_property is now deprecated in favor of Annotation.get_property
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨N New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants