Skip to content

Refactor/oos handling#102

Merged
voorhs merged 55 commits intodevfrom
refactor/oos-handling
Jan 24, 2025
Merged

Refactor/oos handling#102
voorhs merged 55 commits intodevfrom
refactor/oos-handling

Conversation

@voorhs
Copy link
Collaborator

@voorhs voorhs commented Jan 23, 2025

мне осталось дописать тесты на стратификацию и обновить датасеты на хагинг фейсе

что изменилось в этом огромном пр:

  • главное: теперь OOS семплы не хранятся в отдельном сплите а распределяются по всем сплитам, игнорируя train_0 и val_0 (сплиты для скоринг ноды)
  • тоже важно: теперь метки OOS семплов не конвертируются в [0,0,0,0], а остаются None (так сделал чтобы была консистентность между хранением OOS семплов для мультикласс и мультилейбл датасетов)
  • почти не важно: теперь при чтении из json мультилейбл метки должны быть уже закодированы в one hot
  • еще я поменял тайпинг меток (до этого был только LabelType), теперь у нас целый выбор из SimpleLabel, MultiLabel, ListOfLabels и так далее. пользуйтесь на здоровье в своем коде!
  • данные для тестов обновлены, на хагингфейсе тоже, так что у вас могут падать тесты на ваших ветках простите я случайно :(
  • еще успел обновить парочку других датасетов на хагингфейсе

@voorhs voorhs requested review from Samoed and truff4ut January 23, 2025 00:07
"Ensure that each label falls within the valid range of 0 to `n_classes - 1`."
)
raise ValueError(message)
if isinstance(self.label, list):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Кое-где встречаются такие проверки. Мб здесь использовать специальные типы, которые ты добавил

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

да, наверное стоит потом так и делать

в следующих пр и ревью будем учитывать

"Please provide at least one valid label."
)
raise ValueError(message)
if any(lab not in [0, 1] for lab in self.label):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Точно это сокращение нужно?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

не понял вопрос

Copy link
Collaborator

Choose a reason for hiding this comment

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

Я про lab

Copy link
Collaborator

Choose a reason for hiding this comment

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

Точно нужны эти исключения? Не можем просто ValueError кидать

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

они в тестах у нас используются

и чтобы одно и то же сообщение каждый раз не переопределять

msg = "JinoosDecision is compatible with single-label classification only"
raise WrongClassificationError(msg)
self.n_classes = len(set(labels).difference([-1]))
self._n_classes, multilabel, contains_oos = self._validate_inputs(scores, labels)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Не хочешь в декоратор эти строчки вынести?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

честно хотел бы но денис и рома говорили что декораторы использовать плохо

мб в будущем как-то исправим это дублирование кода

Copy link
Member

Choose a reason for hiding this comment

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

Их не плохо использовать, для валидации норм. Просто раньше была какая-то часть логики в них и это было не удобно

from autointent.custom_types import ListOfLabels

LABELS_VALUE_TYPE = list[LabelType] | npt.NDArray[Any]
LABELS_VALUE_TYPE = ListOfLabels
Copy link
Collaborator

Choose a reason for hiding this comment

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

ALL_CAPS + UpperCamelCase

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

создал issue, потом исправим

MultiLabel = list[int]
SimpleLabelWithOOS = SimpleLabel | None
MultiLabelWithOOS = MultiLabel | None
ListOfLabels = list[SimpleLabel] | list[MultiLabel]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Мб LabelList?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Мне кажется ListOfLabels понятнее

def _separate_oos(self, dataset: HFDataset) -> tuple[HFDataset, HFDataset]:
in_domain_ids = []
out_of_domain_ids = []
for i, sample in enumerate(dataset):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Мб попробовать избавиться от этого цикла через map(with_indices=True)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

прикольно, не знал что такое есть) запомню на будущее, а тут пока пусть останется

@voorhs voorhs mentioned this pull request Jan 24, 2025
@voorhs voorhs merged commit 1824ce3 into dev Jan 24, 2025
21 checks passed
@voorhs voorhs deleted the refactor/oos-handling branch January 24, 2025 11:40
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.

3 participants