mostly_tracked, partially_tracked, mostly_lost not granular enough

[ghost]

0-20%, 20-80%, and 80+% are useful, but for complicated scenes, most everything falls in the "mostly tracked" bucket. Can we get a few more buckets? Maybe one for each 10% or 20% increment? I can't imagine this would be difficult in the code, so if you point the way, I'm willing to implement it and submit a PR myself.

[cheind]

Hey Pavel!

I believe these categories have been introduced in the following paper

@inproceedings{li2009learning,
  title={Learning to associate: Hybridboosted multi-target tracker for crowded scene},
  author={Li, Yuan and Huang, Chang and Nevatia, Ram},
  booktitle={2009 IEEE Conference on Computer Vision and Pattern Recognition},
  pages={2953--2960},
  year={2009},
  organization={IEEE}
}

and have been reported in many follow-up papers. While I believe its useful to add fine-grained steps for inspection, for publications we still need those exact categories. Anyways, the code is in metrics.py

py-motmetrics/motmetrics/metrics.py

Line 465 in 411f4e5

def partially_tracked(df, track_ratios):

Each category is implemented as a separate metric that can be selected for computation.

[ghost]

So you don't want to add more to the master branch? If I want to implement a version for myself that gets more granular, it's a matter of just defing a few more functions and then simple_add_func.append()ing them to the list?

[cheind]

So you don't want to add more to the master branch?

Sorry, I didn't mean to say that. I just wanted to mention that the current behavior should persist.

If I want to implement a version for myself that gets more granular, it's a matter of just defing a few more functions and then simple_add_func.append()ing them to the list?

Yes, basically that's it. The name of the arguments of the function is relevant. I.e they refer to dependencies of other functions.

[ghost]

Okay, I went and added the metrics I want (didn't realize I needed to change create() at the time), but then I realized the pd.Series all these annoyingly-many, nearly-identically-named functions rely on is exposed, so I can just use queries on it like track_ratios[(track_ratios >= i*0.1) & (track_ratios < (i+1)*0.1)].count() to get the counts I want and not pollute your list with a bunch of extra metrics.

[ghost]

Okay, no no no no. I've spent some time playing with the motmetrics.apps.eval_motchallenge script like so:

I found where it lives on disk so I can make additions. I'm importing matplotlib.pyplot and trying to use hist on the track_ratios Series. Trouble is I can't figure out how to get access to that Series because the code is really convoluted. No idea why it was written as a set of functions that have to be registered to the metrics.MetricsHost object instead of just functions of that class. There appear to be a ton of checks in there, but I don't understand them.

If I try adding track_ratios to the metrics list before the create_many call in eval_motchallenge, then I get errors:

Traceback (most recent call last):
  File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/runpy.py", line 194, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/usr/local/lib/python3.8/site-packages/motmetrics/apps/eval_motchallenge.py", line 130, in <module>
    main()
  File "/usr/local/lib/python3.8/site-packages/motmetrics/apps/eval_motchallenge.py", line 124, in main
    summary = mh.compute_many(accs, names=names, metrics=metrics, generate_overall=True)
  File "/usr/local/lib/python3.8/site-packages/motmetrics/metrics.py", line 295, in compute_many
    partials.append(self.compute_overall(details, metrics=metrics, name=names))
  File "/usr/local/lib/python3.8/site-packages/motmetrics/metrics.py", line 231, in compute_overall
    cache[mname] = self._compute_overall(partials, mname, cache, parent='summarize')
  File "/usr/local/lib/python3.8/site-packages/motmetrics/metrics.py", line 324, in _compute_overall
    for depname in minfo['deps_m']:
TypeError: 'NoneType' object is not iterable

If I try calling the track_ratios function on the MetricsHost, I get errors because I don't know which parameters to pass.

Why is this so hard? Why is the code written in such a way that I can't actually access a variable that according to the table in the readme is exposed? How do I actually get my hands on the track_ratios Series so I can plot my histogram?

[ghost]

Slight progress: If I add simple_add_func.append(track_ratios), conspicuously missing from metrics.py, then I don't get the NoneType error anymore. I'll see whether I can parlay this into success.

[ghost]

Nope, comes back NaN. I don't get it.

[ghost]

Okay, I've gone spelunking and found that this is the line where the track_ratios get lost in metrics.py:

partials = [pd.DataFrame(OrderedDict([(k, i[k]) for k in metrics]), index=[name]) for i, name in zip(partials, names)]

The Series with the name track_ratios comes out as an OrderedDict, but it seems that when you make a DataFrame from that, it gets populated with NaN because it isn't capable of having an entire Series at that array location.

I had to go about 12 misdirections deep to figure this out. Clearly the repetitious compute functions in metrics.py are not intended to surface a Series. But how else can I get at the Series?

minfo['fnc'](df_map, *vals) shows me I can call the track_ratios function with the right DataFrameMap, but I don't want to have to figure out how to generate that. And *vals is going to contain the deps for this function, which involves obj_frequencies.

The idea is all this stuff is found for me with calls to the compute functions because it keeps track of dependencies. Really the compute functions need to be able to return more than just single numbers.

[ghost]

I'm seeing now that all that registration is so that you can check for those dependencies. But in order for that to work dependent functions have to be specified with a name identical to the param name. Why not enforce that all function dependencies are made explicit and skip the confusing and verbose inference logic, function registration, and subtle 'this has to have the same name as that' rule?

"Explicit is better than implicit" -import this poem

I'm beginning to get inspired to refactor this code. How open are you to supporting me in such an undertaking?

[cheind]

Hey Pavel,

sorry for the inconvenience. It's true that the registration is implicit through argument names. That's for historic reasons. I'm open to refactoring this part of the code basis iff the following constraints apply

• @jvlmdr and @Helicopt should be involved in the design and review of the PR. Both have contributed to metrics.py in the past. @jvlmdr and @Helicopt would you be available to contribute?
• we need to support caching of intermediate results, so that dependencies are not evaluated multiple times.
• pytest code coverage should at least be what it is now and performance should be at least of what it is now.

Best,
Christoph

[ghost]

Sorry, I get kind of snarky when I'm frustrated.

I tried some refactoring for a few hours myself but ran in to several things where I just had no idea what the intention was, so I had to stop. I definitely need buy-in and help from you guys if I'm to make headway. And this code definitely needs help. It's too long, there are too many functions that seem to be doing the same thing, and there's some crazy local()/global() implicit weirdness going on.

In essence, it seems pretty easy to add single-number metrics now in a targeted fashion (if your metric depends on an already-defined datastructure/function), but defining new ones or surfacing more complex ones is pretty confusing. Seems like you have been the leaders at MOT metics for years, so we can salve a lot of pain at once if this is addressed. Otherwise someone else will eventually come along and usurp you.

[Helicopt]

well, i'm going to have a look.

[Helicopt]

If one want to add a new metric, firstly, a function name and function instance should be registered in 'def create' in metrics.py; secondly, to accelerate calculation, another function instance with same name + '_m' suffix should be defined, in this function the per video results will be provided, so we can make some easy combination instead of compute something from the beginning, for example, we don't need to re-calculate IDF1 for the combination for the all videos (this is time-consuming in an early version), but just do some simple add operations for the results of each video.

[pavelkomarov]

Why is there this '_m' suffix business? It's not easy to understand.

[Helicopt]

simple_add_func is actually a default definition of MetricName_m, because a lot of basic metrics can be combined through a weighted sum way, and it is not beautiful to repeat the same things for a dozen times. In fact, each metric should be viewed as a sub-class which inheriting a common parent-class, it will be a more reasonable framework, however, it is also largely change the current code so I did not doing so then.

[Helicopt]

Why is there this '_m' suffix business? It's not easy to understand.

for example, in the original design, when we compute the overall IDF1 score, we combine 7 videos into one very large video, and recaculate the IDF1 for this large fake video. actually we have the IDF1 score computed for each of the 7 video, a more efficient way is to find out the relationship of the overall IDF1 and the IDF1-s for each video. so we need to defined another metric function, for a fast merging. '_m' means merge.

[pavelkomarov]

I still don't fully understand. And having to push all these functions in to a list, iterate the list to create closures, shove those closures in locals(), and then pull them out of globals() in MetricsHost is about the strangest possible way to pass these things around. If they should properly be subclasses, make them subclasses. But I hesitate to think that's right either, because I'm not a big fan of OOP for little stuff; it gets in the way by constantly sending me round asking "what is this thing again?"

[Helicopt]

doing this is to automatically infer the dependency graph. some python tricks are used. i was just following and extending the idea. of course turning all into classes will make it easier to understand (if i was the initial designer, i definitly will use class strure) . but now it will be a big change and will take time to rebuild the framework.

[ghost]

Can it be undertaken anyway? I spent a whole day trying to access a series that is supposed to be exposed only to realize I can't.

[cheind]

From my point of view refactoring that part of the code would make a good investment into the future of py-motmetrics. I know of several metrics to be published in the short term that eventually should make it into py-motmetrics.

Maybe we can trace the current way the metrics are computed. @Helicopt would you mind giving us a head-start? Its quite a long time for me that I actually worked on the part of the code :(

[ghost]

I'm not convinced making dependent metrics subclasses of the ones they depend on is the right way to encode that relationship. Making each one a class seems like a recipe for really verbose code to me. It will be clearer, but still hard to understand. Maybe a dictionary that encodes who depends on whom is sufficient? It's no more annoying to know you have to add an entry to a dictionary than to know you have to inherit from certain things, and it's maybe even more explicit than subclassing because you can see all dependency relationships in one, short place.

For a large enough project with enough metrics, though, subclassing may be better, because then each piece of code can be smaller, and new contributors can just ignore almost all metrics because they're just analogous to whichever example they choose to look at. I'm sure an elegant way to evaluate parents first and pass results exists.