Changing the hashing methodology for cache folder creation of models. #481
Conversation
… QNN compilation not included yet. Cache folder mechanism has been modified to have a parent directory for a model based on the architecture that we retrieve from the model config. The hash calculation for the ONNX export now incorporates all model kwargs as well as export kwargs and parameters. the parameters that were used to create the hash also gets dumped as a serialized JSON file in the ONNX folder, the same happens for the compile parameters inside the respective qpc folder. Signed-off-by: Dhiraj Kumar Sah <quic_dhirajku@quicinc.com>
quic-dhirajku
requested review from
quic-rishinr,
ochougul,
quic-hemagnih and
quic-amitraj
as code owners
QEfficient/base/modeling_qeff.py
Outdated
| @@ -5,7 +5,7 @@ | |||
| # | |||
| # ---------------------------------------------------------------------------- | |||
|
|
|||
| import hashlib | |||
| # import hashlib | |||
There was a problem hiding this comment.
commented code.
Make sure that commented code is not there in ready to review PRs.
QEfficient/base/modeling_qeff.py
Outdated
| self.model_params.update(kwargs) | ||
| self.model_params["config"] = self.model.config.to_diff_dict() | ||
| self.model_params["_transform_names"] = self._transform_names() | ||
| self.compile_params = {} |
There was a problem hiding this comment.
initialize this only when compile is called.
No point in creating this dictionary if user not calling compile.
QEfficient/base/modeling_qeff.py
Outdated
| self.model_params = {} | ||
| self.model_params.update(kwargs) |
There was a problem hiding this comment.
Better to do self.model_params = copy.deepcopy(kwargs)
This lets other methods mutate kwargs.
Otherwise we would need to ensure that no other method mutates the kwargs.
QEfficient/base/modeling_qeff.py
Outdated
| if export_kwargs is not None: | ||
| self.model_params.update(export_kwargs) | ||
| if onnx_transform_kwargs is not None: | ||
| self.model_params.update(onnx_transform_kwargs) |
There was a problem hiding this comment.
One liners are better
self.model_params.update(export_kwargs) if export_kwargs is not None else None
self.model_params.update(onnx_transform_kwargs) if export_kwargs is not None else None
QEfficient/base/modeling_qeff.py
Outdated
| self.model_params["output_names"] = output_names | ||
| self.model_params["dynamic_axes"] = dynamic_axes |
There was a problem hiding this comment.
Better to keep them in one more level as
self.model_params["export_params"] = export_params
And add all exports params in export_params which is another dict.
Makes the dumped JSON readable by user.
QEfficient/base/modeling_qeff.py
Outdated
| model_params_json = export_dir / "model_params.json" | ||
| with open(model_params_json, "w") as fp: | ||
| json.dump( | ||
| { | ||
| "model_params": [ | ||
| {k: make_serializable(self.model_params[k]) for k in sorted(self.model_params.keys())} | ||
| ] | ||
| }, | ||
| fp, | ||
| indent=4, | ||
| ) |
There was a problem hiding this comment.
Dumping should happen after export.
If model errors out during export and we still dump the json, it does not make sense
|
|
||
| self.pretrained_model_name_or_path = kwargs.get("pretrained_model_name_or_path", None) | ||
| # self.pretrained_model_name_or_path = kwargs.get("pretrained_model_name_or_path", None) |
Signed-off-by: Dhiraj Kumar Sah <quic_dhirajku@quicinc.com>
Signed-off-by: Dhiraj Kumar Sah <quic_dhirajku@quicinc.com>
There was a problem hiding this comment.
To control the user should not initialize the model directly using the init of modeling class. Please use Metaclass Control to control the flow and print the warning or error. For example-
class NoInitMeta(type):
def __call__(cls, *args, **kwargs):
raise RuntimeError("Use `from_pretrained` to create an instance.")
class MyModel(metaclass=NoInitMeta):
def __init__(self, data):
self.data = data
@classmethod
def from_pretrained(cls, path):
instance = object.__new__(cls)
instance.__init__(f"Loaded from {path}")
return instance
More about this you can read from here- https://stackoverflow.com/questions/100003/what-are-metaclasses-in-python.
Put that meta class in the utils and use this for all the modelling class.
| @@ -357,6 +388,19 @@ def _compile( | |||
| logger.info(f"Running compiler: {' '.join(command)}") | |||
| try: | |||
| subprocess.run(command, capture_output=True, check=True) | |||
|
|
|||
| # Dumping compile paramters in a JSON file after successful QPC compilation | |||
There was a problem hiding this comment.
Remove all the code related to compile_param_json from here including dumping and handle these inside the decorator dump_qconfig. Lets keep the base methods clean.
| model_params_json = export_dir / "model_params.json" | ||
| with open(model_params_json, "w") as fp: | ||
| json.dump( | ||
| { |
There was a problem hiding this comment.
Same like compile create a decorator and handle all these param updates and dumping inside that.
| export_params["output_names"] = output_names | ||
| export_params["dynamic_axes"] = dynamic_axes | ||
|
|
||
| self.model_params["export_params"] = export_params |
There was a problem hiding this comment.
Handle in decorator. Lets keep our base methods clean.
There was a problem hiding this comment.
Agree, lets write decorator implementation to handle this.
Detaching hash function for model cache path calculation. changes for QNN compilation not included yet.
Cache folder mechanism has been modified to have a parent directory for a model based on the architecture that we retrieve from the model config. The hash calculation for the ONNX export now incorporates all model kwargs as well as export kwargs and parameters. the parameters that were used to create the hash also gets dumped as a serialized JSON file in the ONNX folder, the same happens for the compile parameters inside the respective qpc folder.