CU-8694gzbn3 k fold metrics

[mart-r]

Add k-fold metrics.

In order to do that, I had to add a sort of state-chaching system.
Because in order for this to make sense, we need to fine-tune using k-1 folds and get metrics for the kth fold. But before moving to the next fold we need to restore to the previous state. Otherwise we'd be getting metrics on stuff we've trained on.

Seems kind of ready, but would definitely need some comments.

EDIT:
In light of issues with tests running (which I never got a full understanding of why it was happening - read more in comment below) there were these additional changes along with this PR:

• Removing old/archived/legacy tests
  • These tests were never run since the folders which they were in (tests/medmentions/ and tests/archive_tests) didn't have __init__.py files
  • However, they would be picked up with the new system (more about this below)
• Changes to main workflow
  • Split the unit tests into 2
    • By listing all the test files and only running on half of them at a time
      • This is done automatically during GHA time
    • This ensures all the tests are run and that they are successful
  • Changed timeout to 25 minutes per half of the tests being run

[tomolopolis]

Task linked: CU-8694gzbn3 Improve metrics (i.e k-fold)

[baixiac]

Btw, I have observed memory increase after consecutive training sessions even using an initialised cat instance in between so every so often, I had to restart our training server to bring the memory allocation back down to normal. So possible, the need to split tests into two runs to make the whole suite pass coincidently indicates a code smell in the cat.train_supervised and/or cat.train. I haven't observed a similar issue on transformer_ner.train which uses a different training paradigm.

[baixiac]

Better to have docstring for the arguments, e.g, for the meaning of nr_of_folds, is it num_of_folds?

[mart-r]

Btw, I have observed memory increase after consecutive training sessions even using an initialised cat instance in between so every so often, I had to restart our training server to bring the memory allocation back down to normal. So possible, the need to split tests into two runs to make the whole suite pass coincidently indicates a code smell in the cat.train_supervised and/or cat.train. I haven't observed a similar issue on transformer_ner.train which uses a different training paradigm.

Thanks for letting me know! I've not observed that explicitly, but it does sound like something that could very well cause the issue here.
I've got a feeling the issue is memory related, but a) the tests run fine locally; b) there is no documented memory usage limit for GHA workflow as far as I can tell; and c) there is no error describing the reason the workflow is being cancelled.

In any case, there may very well be a memory leak, I suppose. Or for some reason some dereferenced variables may not get garbage collected. In any case, since you've observed something like this, it may warrant for me to look into it a little further to see if/when/why this may be happening. And potentially improve the memory situation. Especially if there is an actual memory leak.

[baixiac]

The GH runner specs can be found here. Ofc, you may be able to get a better machine on CREATE and use it as a self-hosted runner.

[mart-r]

The GH runner specs can be found here. Ofc, you may be able to get a better machine on CREATE and use it as a self-hosted runner.

Thanks! My previous googling only led to the billing page where they described how many minutes/hours and how many concurrent jobs can be run.
So I hadn't seen this one.

[baixiac]

Is that possible to include standard deviations for metrics in the returned stats as additions to the means? Users may want to know whether the model performs consistently across different folds.

[tomolopolis]

agreed - that would be nice to have. New ticket here CU-8694vbw6y

[tomolopolis]

on the test issues - if a CREATE GHA runner would be useful we can provision, but test splitting in half, where the total number of tests is now more than , i.e. we're 2 tests twice is a little odd.
To summarise there's potentially:

• a memory leak in the cdb code
• state between tests causing one or more tests to hang

Fixing the above is outside the scope of this PR, and is likely an issue that's been around for a while. Lets prioritise looking into this though.

[tomolopolis]

lgtm

[tomolopolis]

There's various methods that perhaps should be moved out of the cdb module now this utils module exists. I was thinking the cdb merge functionality that Adam wrote a while ago. Thoughts?

[mart-r]

The CDB merge functionality is in medcat.utils.cdb_utils:
https://github.com/CogStack/MedCAT/blob/master/medcat/utils/cdb_utils.py

However, I added the CDB state code in a different utility module because I felt there was enough state-related code to justify having this separately.
With that said, there would also be valid arguments for keeping all CDB-related utilities together in one utility method. So I'm open to making this change as well.

Furthermore, there may very well be more CDB-related things that are currently in the medcat.cdb module that would better be suited in the medcat.utils.cdb_utils module. The former is certainly on the larger side as a whole.
I think a new CU task could be in order.

[mart-r]

on the test issues - if a CREATE GHA runner would be useful we can provision, but test splitting in half, where the total number of tests is now more than , i.e. we're 2 tests twice is a little odd. To summarise there's potentially:

• a memory leak in the cdb code
• state between tests causing one or more tests to hang

Fixing the above is outside the scope of this PR, and is likely an issue that's been around for a while. Lets prioritise looking into this though.

I did try to look into this briefly at the time (when Xi brought up the memory issues). I wasn't able to fully figure out the procedure in which the memory issue arose. But I can try and dedicate more time to figure this out in the context of another task.

[baixiac]

Yes, I can confirm the "memory issue" on consecutive trainings has been around for a long time and is beyond the scope of this PR.

[mart-r]

Yes, I can confirm the "memory issue" on consecutive trainings has been around for a long time and is beyond the scope of this PR.

I've created a CU task:
https://app.clickup.com/t/8694vdx65
If you have any details as to what situations these issues arise, it would be great if you could write them down under there somewhere.

[baixiac]

Oh sure. The simple way to replicate it is to create a cat object, and call cat.train or cat.train_supervised in a loop.

[mart-r]

Oh sure. The simple way to replicate it is to create a cat object, and call cat.train or cat.train_supervised in a loop.

I tried that last time (only with supervised training) and wasn't able to meaningfully change the RAM usage. Perhaps it was the nature of my MCT export (synthetic data, all annotations marked as correct, only 15 documents, no meta cats). Not fully sure.

In any case, I'll be sure to take a look soon.

[baixiac]

One thing I need to mention is I didn't wait long enough to see whether it will cause OOMs. So another slim chance is pymalloc was holding a cache of GC-ed objects and didn't return memory to OS soon enough to achieve a full GC.