Skip to content

feat(medcat): Get Stats: harder, better, (probably not) faster, stronger - #606

Open
adam-sutton-1992 wants to merge 15 commits into
mainfrom
feat_new_get_stats
Open

adam-sutton-1992 wants to merge 15 commits into
mainfrom
feat_new_get_stats

Conversation

@adam-sutton-1992

Copy link
Copy Markdown
Contributor

Hihi,

A new world for get_stats.

Adds three new character metrics:

  • Character IoU (which is a pretty standard definition of a metric).
  • Golden Character IoU. Which is I believe an interesting metric for us. Essentially the character IoU of all CUIs that have a label. Something of a relation to the recall - I'd say.
  • Cohen's Kappa. Used often for measuring inter-annotator agreement. However can be used as a metric of classificiation.

Adds two new "modes":

  • "Perfect" linking. Effectively a measure of the performance of the NER step. This doesn't use a perfect linker. It just hacks all labels and predictions to have the same fake cui "NER".
  • "Perfect" ner. A measure on the linking step. This uses a the NER component that cheats, and thus is tasked with linking only.
  • Obviously the full pipeline is also in there.

Minor changes:

  • Returns a StatsCalculator object that can be explored for per cui and overall metrics, along with raw stats.
  • Also counts the occasions where no tokens are found for a label span. This results in a false negative, as the model cannot possibly predict that entity. But it also counts the number of these, a decent metric for the tokenizer.
  • I've also tested and adapted KFold so the metrics are identical to what they were previously. I've made minimal structural changes during testing and they all passed.
  • I hard coded testing for the Stats, and hand calculated what metrics came out should.

I'm a bit unhappy with the naming of the pydantic structure of "RawStats", "ProjectStats", "ModeStats"... It kind of makes sense but is a bit sloppy when reusing it.

@mart-r

mart-r commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

NOTE:
Docs build failures are because of my work on #607 - in order to build there, I need to change the reference to .readthedocs.yaml in settings on RTD side. And this bricks everything other than my PR. I wanted to only change it temporarily (for now) but because there were issues with the docs build I kept it on the PR-specific path for a while. Which is why it failed here.
I should be able to change it back and rebuild the docs with the correct (for this PR) path.

@mart-r mart-r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think this is definitely a step in the right direction!
The setup looks good, the output seems easy to use.
And there's still the option to get the same sort of output.

I don't really think we can introduce a breaking change in the manner that you're doing it here due to unkown downstream effects.

So I'd say this thing (i.e the new returned object) needs to be in its own method and the get_stats needs to use this and unwrap the output (like you've done in various bits).

There's a few nagging things.
But also a few things that I think would need to change.
A few structures I'd like to be defined more rigidly (rather than just dict or predefined strings).
A few bits where I feel like we could easily split out the longer methods into smaller ones.
And then there's a matter of documentation in a few places.
And then one place where I asked for a feature for print output stream.

Comment thread medcat/medcat/stats/stats.py
Comment thread medcat/medcat/stats/stats.py
Comment thread medcat-v2/medcat/stats/stats.py Outdated
Comment thread medcat-v2/medcat/stats/stats.py Outdated
Comment thread medcat/medcat/stats/stats.py
Comment thread medcat-v2/medcat/stats/stats.py Outdated
Comment thread medcat-v2/medcat/stats/stats.py Outdated
Comment thread medcat/medcat/stats/stats.py
Comment thread medcat-v2/medcat/stats/stats.py Outdated
Comment thread medcat/medcat/stats/stats.py

@mart-r mart-r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few doc strings I'd like to see, plus removal of added comments from test_kfold.py.

And the project + aggregate issue that's still present. Would be nice to clean that up, but I don't think it's high priority. Can easily leave as is.

The rest is more or less just nagging.

Comment thread medcat-v2/tests/stats/test_kfold.py Outdated
Comment thread medcat-v2/medcat/stats/stats.py Outdated
Comment thread medcat-v2/medcat/stats/stats.py Outdated
Comment thread medcat/medcat/stats/stats.py
Comment thread medcat-v2/medcat/stats/stats.py Outdated

def _update_project_stats(
self,
project_state: ModeStats,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAG[Proj+Aggr]: Same here, perhaps a list then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, as a list.

Comment thread medcat/medcat/stats/stats.py
Comment thread medcat/medcat/stats/stats.py
Comment thread medcat/medcat/stats/stats.py
@mart-r

mart-r commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

NOTE:
On top of the above, now needs resynced with master due to folder change in #607

@mart-r mart-r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great!
Thanks for the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants