-
Notifications
You must be signed in to change notification settings - Fork 751
Replace config ValueError with OumiConfigValueError #2325
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
base: idoudali/config-exceptions
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| from omegaconf import MISSING | ||
|
|
||
| from oumi.core.configs.params.base_params import BaseParams | ||
| from oumi.exceptions import OumiConfigValueError | ||
|
|
||
|
|
||
| # Training Params | ||
|
|
@@ -49,7 +50,7 @@ def get_literal_value(self) -> Literal["first_exhausted", "all_exhausted"]: | |
| elif self.value == MixtureStrategy.ALL_EXHAUSTED: | ||
| return "all_exhausted" | ||
| else: | ||
| raise ValueError("Unsupported value for MixtureStrategy") | ||
| raise OumiConfigValueError("Unsupported value for MixtureStrategy") | ||
|
|
||
|
|
||
| @dataclass | ||
|
|
@@ -149,24 +150,28 @@ def __post_init__(self): | |
| """Verifies params.""" | ||
| if self.sample_count is not None: | ||
| if self.sample_count < 0: | ||
| raise ValueError("`sample_count` must be greater than 0.") | ||
| raise OumiConfigValueError("`sample_count` must be greater than 0.") | ||
| if self.mixture_proportion is not None: | ||
| if self.mixture_proportion < 0: | ||
| raise ValueError("`mixture_proportion` must be greater than 0.") | ||
| raise OumiConfigValueError( | ||
| "`mixture_proportion` must be greater than 0." | ||
| ) | ||
| if self.mixture_proportion > 1: | ||
| raise ValueError("`mixture_proportion` must not be greater than 1.0 .") | ||
| raise OumiConfigValueError( | ||
| "`mixture_proportion` must not be greater than 1.0 ." | ||
| ) | ||
|
Comment on lines
151
to
+162
|
||
|
|
||
| if self.transform_num_workers is not None: | ||
| if isinstance(self.transform_num_workers, str): | ||
| if not (self.transform_num_workers == "auto"): | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| "Unknown value of transform_num_workers: " | ||
| f"{self.transform_num_workers}. Must be 'auto' if string." | ||
| ) | ||
| elif (not isinstance(self.transform_num_workers, int)) or ( | ||
| self.transform_num_workers <= 0 | ||
| ): | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| "Non-positive value of transform_num_workers: " | ||
| f"{self.transform_num_workers}." | ||
| ) | ||
|
|
@@ -176,7 +181,7 @@ def __post_init__(self): | |
| self.dataset_kwargs.keys() | ||
| ) | ||
| if len(conflicting_keys) > 0: | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| "dataset_kwargs attempts to override the following " | ||
| f"reserved fields: {conflicting_keys}. " | ||
| "Use properties of DatasetParams instead." | ||
|
|
@@ -270,23 +275,23 @@ def __post_init__(self): | |
| if not all( | ||
| [dataset.mixture_proportion is not None for dataset in self.datasets] | ||
| ): | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| "If `mixture_proportion` is specified it must be " | ||
| " specified for all datasets" | ||
| ) | ||
| mix_sum = sum( | ||
| filter(None, [dataset.mixture_proportion for dataset in self.datasets]) | ||
| ) | ||
| if not self._is_sum_normalized(mix_sum): | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| "The sum of `mixture_proportion` must be 1.0. " | ||
| f"The current sum is {mix_sum} ." | ||
| ) | ||
| if ( | ||
| self.mixture_strategy != MixtureStrategy.ALL_EXHAUSTED | ||
| and self.mixture_strategy != MixtureStrategy.FIRST_EXHAUSTED | ||
| ): | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| "`mixture_strategy` must be one of " | ||
| f'["{MixtureStrategy.FIRST_EXHAUSTED.value}", ' | ||
| f'"{MixtureStrategy.ALL_EXHAUSTED.value}"].' | ||
|
|
@@ -324,12 +329,12 @@ def get_split(self, split: DatasetSplit) -> DatasetSplitParams: | |
| elif split == DatasetSplit.VALIDATION: | ||
| return self.validation | ||
| else: | ||
| raise ValueError(f"Received invalid split: {split}.") | ||
| raise OumiConfigValueError(f"Received invalid split: {split}.") | ||
|
|
||
| def __finalize_and_validate__(self): | ||
| """Verifies params.""" | ||
| if len(self.train.datasets) == 0: | ||
| raise ValueError("At least one training dataset is required.") | ||
| raise OumiConfigValueError("At least one training dataset is required.") | ||
|
|
||
| all_collators = set() | ||
| if self.train.collator_name: | ||
|
|
@@ -339,11 +344,11 @@ def __finalize_and_validate__(self): | |
| if self.test.collator_name: | ||
| all_collators.add(self.test.collator_name) | ||
| if len(all_collators) >= 2: | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| f"Different data collators are not supported yet: {all_collators}" | ||
| ) | ||
| elif len(all_collators) == 1 and not self.train.collator_name: | ||
| raise ValueError( | ||
| raise OumiConfigValueError( | ||
| "Data collator must be also specified " | ||
| f"on the `train` split: {all_collators}" | ||
| ) | ||
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.
What's the difference between OumiConfigError and OumiConfigValueError? Should we just use one?