Skip to content
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

BUG: eval crashes for learncurve when labelmap has multi-char label, because eval re-maps the labelmap #664

Closed
5 tasks
NickleDave opened this issue Jun 5, 2023 · 0 comments · Fixed by #665

Comments

@NickleDave
Copy link
Collaborator

In eval we currently re-map the labels so that they all have single characters, so that we do not artificially inflate or deflate the edit distance; see #373

https://github.com/vocalpy/vak/blob/a6c43bc1587e248e5002123270be29dd5f6a4c7a/src/vak/core/eval.py#LL148C1-L151C62

    labelmap_keys = [lbl for lbl in labelmap.keys() if lbl != 'unlabeled']
    if any([len(label) > 1 for label in labelmap_keys]):  # only re-map if necessary
        # (to minimize chance of knock-on bugs)
        labelmap = multi_char_labels_to_single_char(labelmap)

But this now causes a crash when we try to load the validation set a few lines later.

val_dataset = VocalDataset.from_csv(

It happens because we try to make a "temporary" batch so that we can dynamically determine the dataset's shape attribute from the source returned in the batch.
tmp_item = self.__getitem__(tmp_x_ind)

(Do we still need to do this?)

  File "/home/ildefonso/Documents/repos/vocalpy/vak-vocalpy/src/vak/datasets/vocal_dataset.py", line 80, in __init__
    tmp_item = self.__getitem__(tmp_x_ind)
  File "/home/ildefonso/Documents/repos/vocalpy/vak-vocalpy/src/vak/datasets/vocal_dataset.py", line 94, in __getitem__
    lbls_int = [self.labelmap[lbl] for lbl in annot.seq.labels]
  File "/home/ildefonso/Documents/repos/vocalpy/vak-vocalpy/src/vak/datasets/vocal_dataset.py", line 94, in <listcomp>
    lbls_int = [self.labelmap[lbl] for lbl in annot.seq.labels]

I'm not sure why we didn't hit this bug before, e.g. with canary song labeled with integers.

We don't actually want to change the mapping for this purpose. We only want to do it inside validation_step, when we convert labeled timebins to labels (not when we're converting labels to a vector of labeled timebins, as we do when loading samples for this dataset class). This is what we use the to_labels transform for that is currently an attribute of the class.

So I think the right fix for this is as follows, adhering to the principle of least surprise:

  • don't remap inside core.eval, to avoid breaking VocalDataset
  • instead validate the labelmap inside of WindowedFrameClassificationModel.
    • should we do this in the class method or in the init? My first though is in class method to keep from doing validation and having a verbose init, but that would make it possible to instantiate a WindowedFrameClassificationModel "by hand" that calculates a different segment error rate, which we don't want. So validate there
    • Add a logger and log that we are doing this re-mapping of labelmap
  • we want to make it explicit that we have a separate labelmap for eval, so we'll do what we currently do inside core eval, but make this a separate attribute (labelmap_eval) and then use that labelmap with the to_labels transform, that we'll rename to_labelmap_eval to be extra explicit
  • also document what's been done inside the WindowedFrameClassificationModel docstrings
@NickleDave NickleDave changed the title BUG: eval crashes for learncurve when labelmap has multi-char because it changes labelmap BUG: eval crashes for learncurve when labelmap has multi-char label, because eval re-maps the labelmap Jun 5, 2023
NickleDave added a commit that referenced this issue Jun 5, 2023
…Model only, fix #664 (#665)

* Remove re-mapping of labelmap inside core.eval

* Fix WindowedFrameClassificationModel to re-map labelmap for eval

* Add pytest marker 'slow' to ini_options in pyproject.toml

* Mark tests in test_train as slow

* Mark tests in test_core/test_learncurve as slow

* Add logic in tests/conftest.py to sort tests so 'slow' marks run last, and cli option to turn this on

* Fix TEST_INIT_ARGVALS in test_models/test_tweetynet so we don't create spurious error with integer keys in a labelmap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant