Add metric computations

[kaitj]

Proposed changes
This initializes a metrics.py for some functions that act on a list of valid inputs to compute metrics such as mean distance or mean AFID location. Also introduces two new classes:

1. AfidDistance, which contains information regarding the AFID pair used for computation, as well as both the Euclidean distance and distance along each spatial component. This was made flexible to be able to handle computation between any two AFIDs
2. AfidDistanceSet, which contains information regarding the valid AfidSets used for computation, returning distances between corresponding pairs of AFIDs for a full AfidSet.

As is, this is ready for a review, but open to suggestions of other metrics to implement.

Types of changes
What types of changes does your code introduce? Put an x in the boxes that apply

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Other (if none of the other choices apply)

Checklist
Put an x in the boxes that apply. You can also fill these out after creating the PR. If you are unsure about any of the choices, don't hesitate to ask!

• Changes have been tested to ensure that fix is effective or that a feature works.
• Changes pass the unit tests
• Code has been run through the poe quality task
• I have included necessary documentation or comments (as necessary)
• Any dependent changes have been merged and published

Notes
All PRs will undergo the unit testing before being reviewed. You may be requested to explain or make additional changes before the PR is accepted.

[kaitj]

@ataha24 @jclauneuro - Are there other useful metrics related to AFIDs that we might want to implement prior to merging this (following a review)?

[codecov]

Codecov Report

Merging #35 (a59440e) into main (0298870) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##              main       #35    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           10        12     +2     
  Lines          654       760   +106     
==========================================
+ Hits           654       760   +106     

Components	Coverage Δ
afids_utils/afids.py	∅ <ø> (∅)
afids_utils/ext	100.00% <ø> (ø)
afids_utils/metrics.py	100.00% <100.00%> (∅)
afids_utils/transforms.py	100.00% <ø> (ø)

[tkkuehn]

I'm not totally sure about this API but I'm also not sure that any of my suggestions are really any better. I think having a concrete use-case for this would help clarify. Maybe it would be useful for visualizations or a way to calculate AFLEs from a set of AfidSets? Anyway I think it bears a little discussion before merging

[kaitj]

I am in the process of rewriting this a little bit (and may benefit from a call to discuss this more), but one thought that came out of this - if we are storing information regarding the afids pair that computes the distances, then all the current distance parameters (x, y, z) could just be a property computed from afids_pair parameter. Which then adds an additional question of should compute_distance just be a method of AfidDistance rather than function in metrics.py? The one thought was to keep all of the potential metric computations in one place rather than have a class for each metric with its own methods

For reference, here is a screenshot of the current state (and what is causing me to go in circles a litlte bit):

[tkkuehn]

Okay, I think taking a step back and thinking about use cases for this might be helpful.

1. Simplest is the AFIDs Validator-style rendering of differences between a rater and a ground truth. The api might look like:

   distances = [
       AfidDistance(ground_truth_position, rater_position)
       for ground_truth_position, rater_position
       in zip(ground_truth_afids.afids, rater_afids.afids)
   ]

   I think this is okay, but maybe we might want an AfidDistanceSet or something to make it a bit easier.
2. Then maybe you might want mean AFLE from a bunch of rater AfidSets. That could look like:

   mean_set = AfidSet(
       afids=[
           AfidPosition(x=sum([rater_set.afids[idx].x for rater_set in rater_sets]) / len(rater_sets), ...)
           for idx, afid in rater_set[0].afids
       ],
       ...
   )
   mean_afle = [
       sum(
           [AfidDistance(rater_set.afids[idx], mean_set.afids[idx]).distance for rater_set in rater_sets]
       ) for idx, afid in rater_set[0].afids
   ]

   This also seems okay (calculating the mean could also be made easier by having some facility to handle a set of AfidSets or something I guess).

So I guess ultimately making an AfidDistance just take two AfidSets (I wouldn't make them a tuple) seems usable enough.

[kaitj]

I agree with most of this, though I would think we might still want to have AfidDistance be as flexible as possible (e.g. able to work on a single pair of points), and have a different AfidSetDistance for valid sets or something like that. I guess my question is more along the lines of should these still be in afids.py or should these really just be implemented in metrics.py instead. Even the calculations for like mean_set and mean_afle that you noted could be a class method.

EDIT: After working through this a little more, I think I am okay with AfidDistance being its own class with no explicit compute_distance method / function. That said, I think some of the metrics as you noted should be made explicit under metrics.py

[tkkuehn]

Yeah I think with this setup compute_distance as an independent function is kind of obsolete. Could also make a get_distance_from method of AfidPosition that takes another AfidPosition and returns an AfidDistance. I think this stuff can probably be in afids.py and leave higher-level metrics that bundle multiple sets/positions in metrics.py, but I don't feel super strongly about it.

[kaitj]

Alright, a slightly bigger refactor from how this was initially implemented. Also realized I never responded to one of your original comments, but I foresee utility for these in the plotting.

This, for the moment just implements (a slight variant) of the metrics you had mentioned. I think I would like mean_distances to also compute along the spatial components, but need to think a little bit more about how to go about that. Can't return an AfidDistance object given how it is currently implemented.

EDIT: Also, as you noted, we should have a discussion about this (next meeting?) prior to merging.

[tkkuehn]

A couple more small changes but this is looking good

[tkkuehn]

Needs a merge from main but otherwise LGTM