Set onnx opset version before model select #37466
Conversation
💊 Build failures summary and remediationsAs of commit 66295e5 (more details on the Dr. CI page):
ci.pytorch.org: 1 failedThis comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.Please report bugs/suggestions on the GitHub issue tracker. This comment has been revised 1 time. |
|
@suffiank - can you elaborate in the description what issue are you trying to solve? |
| @@ -498,9 +498,9 @@ def _export(model, args, f, export_params=True, verbose=False, training=None, | |||
| # If you really know what you're doing, you can turn | |||
| # training=TrainingMode.TRAINING or training=TrainingMode.PRESERVE, | |||
| # (to preserve whatever the original training mode was.) | |||
| _set_opset_version(opset_version) | |||
| _set_operator_export_type(operator_export_type) | |||
There was a problem hiding this comment.
Will this set the original mode of the model (eval vs train) after the export in all scenarios? Just trying to understand if there's any effect of taking these two steps outside the with block. @lara-hdr
There was a problem hiding this comment.
So I noticed the message print even when I set opset 12. Initially I thought the parameter wasn't being respected and an edit was required. Turns out it's just a bad message. I then matched the call sequence as in the pretty export function.
| @@ -43,8 +43,8 @@ def select_model_mode_for_export(model, mode): | |||
| if is_originally_training: | |||
| warnings.warn("You are exporting the model to ONNX while in training mode with " | |||
| "'train' parameter not specified. The model will default to inference mode export. " | |||
| "If you wish to export a training amenable ONNX model, specify train=TrainingMode.TRAIN or " | |||
| "train=TrainingMode.PRESERVE (to preserve the original model state) in torch.onnx.export().") | |||
| "If you wish to export a training amenable ONNX model, specify training=TrainingMode.TRAINING or " | |||
|
@suffiank - can you please add a test to cover this? |
So I already see tests to cover expected opset version. And also model state is preserved after export (eval vs. training). Since this only fixes printing of warning message, it's better without? |
|
Thanks @suffiank. I had also created a similar PR #37397. |
There was a problem hiding this comment.
@houseroad has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@houseroad merged this pull request in d5363e6. |
Set opset version before model select call - which is used to trigger warnings.