-
Notifications
You must be signed in to change notification settings - Fork 114
Pipeline Extraction #1279
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: main
Are you sure you want to change the base?
Pipeline Extraction #1279
Conversation
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
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.
Definitely looks cleaner this way! Leaving comments rather than approving, as I am still getting up to speed with pipelines
PIPELINES: Dict[str, PipelineFn] = { | ||
"sequential": sequential.run_pipeline, | ||
"layer_sequential": layer_sequential.run_pipeline, | ||
"basic": basic.run_pipeline, | ||
"independent": independent.run_pipeline, | ||
} |
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.
If we make pipelines a class satisfying an abstract Pipeline
base class with
@abstractmethod
def run_pipeline(model:PreTrainedModel, dataloader:DataLoader):...
would it avoid needing maps like this or types like
PipelineFn = Callable[[PreTrainedModel, torch.utils.data.DataLoader], None]
in the typing.py file?
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.
I purposefully avoided adding a base class, since I think it adds more infrastructure than is required. Such a class would only have one method, which imho doesn't justify a class definition.
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
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.
I know you're looking for feedback on this, but I'm not sure I understand it enough to approve. I do like the removal of all the try/catch code in GPTQ. Maybe we can have a deep dive session on this next week?
## Purpose ## * Revert the behavior regression introduced as a result of #1114 * When calibrating a model using the `QuantizationModifier`, quantization should be enabled when calibrating ## Changes ## * Remove "disabling quantization" from the calibration forward pass * Add "disabling quantization" to the sequential pipelines in order to continue to disable quantization during calibration for GPTQ and SGPT * When [calibration pipelines become shared between modifiers](#1279), the decision of whether to disabling quantization during calibration will have to be moved to the calibration pipelines themselves. Some work needs to be done to demonstrate that GPTQ and SGPT do not suffer accuracy regression from enabling activation quantization during calibration (in theory, the change should increase accuracy) --------- Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Signed-off-by: Kyle Sayers <[email protected]>
Purpose
TODO
Changes
sequential_epoch_end
callback_modifier
calibration_epoch_end
session.initialize_recipe
session.get_modifiers
ModifierStages
_hf_hook. pre_forward
withalign_module_device
for clarityresolved_mappings_
type hint for clarity_apply_smoothing
on thesequential_epoch_end
andcalibration_epoch_end
_apply_smoothing
function to be called multiple times per session (as is required by sequential pipeline)Testing