-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Pipeline's YAML: syntax validation #2226
Conversation
…e new custom exception classes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! A huge leap forward!
@julian-risch please check if my last finding regarding distillation does not hurt too much. Otherwise we're good to go.
@ZanSara please remove the commented out setconfig calls and change _component_configuration
to _component_config
before merging or in another PR.
@@ -884,7 +884,7 @@ def _get_checksum(self): | |||
"max_seq_len": self.processor.max_seq_len, | |||
"dev_split": self.processor.dev_split, | |||
"tasks": self.processor.tasks, | |||
"teacher_name_or_path": self.teacher.pipeline_config["params"]["model_name_or_path"], | |||
"teacher_name_or_path": self.teacher.name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really the same? I can't find the name field be set anywhere outside a Pipeline. In test_distillation.py it always seems to be None. @julian-risch could you take a quick look on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it's not the same. self.teacher
is a FARMReader
, which inherits name
from BaseComponent
. While pipeline_config
is also inherited from BaseComponent
, its model_name_or_path
has a different meaning than name
. name
is typically Reader
, whereas model_name_or_path
is typically deepset/roberta-base-squad2
but it could also be a local file path. @ZanSara I believe this change needs to be reverted.
Proposed changes:
BasePipeline.validate_config()
BasePipeline.validate_yaml()
HaystackError
,PipelineError
,PipelineValidationError
, etc... (seehaystack/errors.py
)Status:
Closes #1981
Closes #2252
Closes #2246
Closes #600