Skip to content

CnnScorer#210

Closed
riapush wants to merge 51 commits intodevfrom
cnn_scorer_dumper
Closed

CnnScorer#210
riapush wants to merge 51 commits intodevfrom
cnn_scorer_dumper

Conversation

@riapush
Copy link
Collaborator

@riapush riapush commented May 19, 2025

No description provided.

@voorhs voorhs self-requested a review May 21, 2025 08:04
Copy link
Collaborator

@voorhs voorhs left a comment

Choose a reason for hiding this comment

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

очень здорово что все работает, осталось только отшлифовать деталь с тем что не получилось сделать универсальный торч дампер

Comment on lines +183 to +184
with (path / Dumper.containers / "containers.json").open("w") as f:
json.dump(containers, f, ensure_ascii=False, indent=4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут могут быть несериализуемые типы, надо либо try except либо убедиться что все типы сериализуемы

for model_dir in child.iterdir():
with (model_dir / "class_info.json").open("r") as f:
class_info = json.load(f)
module = __import__(class_info["module"], fromlist=[class_info["name"]])
Copy link
Collaborator

Choose a reason for hiding this comment

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

лучше бы использовать importlib

Copy link
Collaborator

Choose a reason for hiding this comment

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

по поводу торч моделей в дампере: раз уж мы накладываем ограничение на объекты nn.Module что они должны иметь метод get_config то тогда надо в нашей библиотеке завести класс обертку для всех nn.Module и дать ему абстрактный метод get_config

а раз уж не получилось полноценно реализовать идею с универсальным дампером для торч моделей тогда лучше определять dump/load для каждой модели отдельно указав dump/load в этой абстрактной обертке для nn.Module

Comment on lines +203 to +216
def get_implicit_initialization_params(self) -> dict[str, Any]:
"""Return default params used in initialization."""
return {
"max_seq_length": self.max_seq_length,
"num_train_epochs": self.num_train_epochs,
"batch_size": self.batch_size,
"learning_rate": self.learning_rate,
"seed": self.seed,
"report_to": self.report_to,
"embed_dim": self.embed_dim,
"kernel_sizes": self.kernel_sizes,
"num_filters": self.num_filters,
"dropout": self.dropout
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

тут нужно пустой словарь вернуть, потому что смысл этого метода не в том чтобы вернуть все дефолтные параметры


def fit(self, utterances: list[str], labels: ListOfLabels) -> None:
self._validate_task(labels)
self._multilabel = isinstance(labels[0], (list, np.ndarray)) # noqa: UP038
Copy link
Collaborator

Choose a reason for hiding this comment

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

этот атрибут не нужно устанавливать, он устанавливается в _validate_task

probs = torch.softmax(outputs, dim=1).cpu().numpy()
all_probs.append(probs)

return np.concatenate(all_probs, axis=0) if all_probs else np.array([])
Copy link
Collaborator

Choose a reason for hiding this comment

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

почему тут может быть пустой выход? если вход изначально пустой?

@voorhs voorhs closed this Jun 16, 2025
@voorhs voorhs deleted the cnn_scorer_dumper branch June 22, 2025 10:29
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

Comments