Adding _ops and _weight_size metadata checks to tests #6996

toni057 · 2022-11-30T13:29:16Z

Continuing on PR6936 where number of operations and model sizes were added, in this PR we are adding the logic for calculating the mentioned metadata to test, and verifying that the values added to metadata correspond to the values hardcoded for weights.

Due to the relatively long run times, we are limiting the solution to default weights only.

cc: @datumbox

cc @datumbox @pmeier

datumbox

Thanks for the work @toni057. Just a few comments:

datumbox · 2022-11-30T14:01:02Z

test/common_extended_utils.py

+detection_models_input_dims = {
+    "fasterrcnn_mobilenet_v3_large_320_fpn": (320, 320),
+    "fasterrcnn_mobilenet_v3_large_fpn": (800, 800),
+    "fasterrcnn_resnet50_fpn": (800, 800),
+    "fasterrcnn_resnet50_fpn_v2": (800, 800),
+    "fcos_resnet50_fpn": (800, 800),
+    "keypointrcnn_resnet50_fpn": (1333, 1333),
+    "maskrcnn_resnet50_fpn": (800, 800),
+    "maskrcnn_resnet50_fpn_v2": (800, 800),
+    "retinanet_resnet50_fpn": (800, 800),
+    "retinanet_resnet50_fpn_v2": (800, 800),
+    "ssd300_vgg16": (300, 300),
+    "ssdlite320_mobilenet_v3_large": (320, 320),
+}


Nit: I think this doesn't belong on the common_extended_utils.py file but rather on the test/test_extended_models.py file.

Sure, can move it there.

datumbox · 2022-11-30T14:02:58Z

test/test_extended_models.py

            else:
                if w.meta.get("num_params") != sum(p.numel() for p in model_fn(weights=w).parameters()):
                    incorrect_params.append(w)
+
+                calculated_ops = get_ops(module_name, model_name, w)


we need to review this logic. This way we initialize the models multiple times. Once on the model_fn call above and once within get_ops(). What we can do is initialize the model once and then use it in both cases.

datumbox · 2022-11-30T17:22:39Z

test/test_extended_models.py

            if module_name == "quantization":
                # parameters() count doesn't work well with quantization, so we check against the non-quantized
                unquantized_w = w.meta.get("unquantized")
                if unquantized_w is not None and w.meta.get("num_params") != unquantized_w.meta.get("num_params"):
                    incorrect_params.append(w)
+
+                # the methodology for quantized ops count doesn't work as well, so we take unquantized FLOPs instead
+                calculated_ops = get_ops(model=None, module_name="models", model_name=model_name, weight=unquantized_w)


We don't have to do this estimation. We can follow the same approach as with the num_params. More precisely:
we fetch the unquantized_w.meta.get("_ops") and confirm that the match what we have here. Basically we reproduce the logic on lines 219-220.

datumbox · 2022-11-30T17:24:10Z

test/common_extended_utils.py

+        return sum(self.flop_counts["Global"].values()) / 1e9
+
+
+def get_ops(model: torch.nn.Module, module_name: str, model_name: str, weight: Weights, h=512, w=512):


Let's assume here that model is not None. Then we don't need the model_name parameter. The module_name is also unnecessary as it can be fetched from the model. More specifically:

>>> m = resnet50() >>> m.__module__ 'torchvision.models.resnet'

datumbox · 2022-11-30T17:24:23Z

test/common_extended_utils.py

+    if model is None:
+        kwargs = {"quantize": True} if module_name == "quantization" else {}
+        model = models.get_model(model_name, weights=weight, **kwargs)


This can go away:

Suggested change

if model is None:

kwargs = {"quantize": True} if module_name == "quantization" else {}

model = models.get_model(model_name, weights=weight, **kwargs)

datumbox · 2022-11-30T17:25:22Z

test/test_extended_models.py

+                # loading the model and using it for parameter and ops verification
+                kwargs = {"quantize": True} if module_name == "quantization" else {}


Not necessary. We already checked it's not quantization above.

datumbox · 2022-11-30T17:26:59Z

test/test_extended_models.py

+                )
+
+            # assert that weight flops are correctly pasted to metadata
+            assert calculated_ops == w.meta["_ops"]


We shouldn't assert like this because it will fail immediately the test without showing us other issues. Instead we should be collecting all issues in one list and showing them to the user. Previously we had incorrect_params which was monitoring issues with the number of parameters. Now that we have more, it's worth switching this into something like incorrect_meta and append to it not only the weight but also the meta name that failed. For example: incorrect_params.append((w, "num_params")).

datumbox · 2022-11-30T17:31:25Z

test/test_extended_models.py

    assert not problematic_weights
    assert not incorrect_params
    assert not bad_names
+    assert weight_size_mb == w.meta["_weight_size"]


Similar to the above. This needs to be asserted properly for all weights. You can use the proposed incorrect_meta to track it as well.

datumbox

Thanks a lot @toni057. Looks great. The comment below is optional.

Let's wait for the tests to see whether there is any randomness, otherwise we should be good.

datumbox · 2022-12-01T11:01:19Z

test/test_extended_models.py

+                    incorrect_meta.append((w, "num_params"))
+
+                # the methodology for quantized ops count doesn't work as well, so we take unquantized FLOPs instead
+                if unquantized_w is not None:


Minor Nit: Since this check is needed for both num_params and _ops we can perhaps do it once for both and simplify the code?

datumbox

LGTM, only one optional Nit below. Your call.

Otherwise we can merge on green CI.

test/test_extended_models.py

Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

Summary: * Adding _ops and _weight_size metadata checks to tests * Fixing wrong ops value * Changing test_schema_meta_validation to instantiate the model only once * moving instantiating quantized models inside get_ops * Small refactor of test_schema_meta_validation logic * Reverting to previous ops value * Simplifying unquantized models logic in test_schema_meta_validation * Update test/test_extended_models.py Reviewed By: datumbox Differential Revision: D41836893 fbshipit-source-id: 9174c95ee1843d972898fcd89c3d4e1697e83bca Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com> Co-authored-by: Toni Blaslov <tblaslov@fb.com> Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

Adding _ops and _weight_size metadata checks to tests

93ac2cf

facebook-github-bot added the cla signed label Nov 30, 2022

datumbox reviewed Nov 30, 2022

View reviewed changes

Toni Blaslov added 3 commits November 30, 2022 16:23

Fixing wrong ops value

414ba8c

Changing test_schema_meta_validation to instantiate the model only once

3bc462f

moving instantiating quantized models inside get_ops

db6e25c

datumbox reviewed Nov 30, 2022

View reviewed changes

Small refactor of test_schema_meta_validation logic

d580171

datumbox reviewed Dec 1, 2022

View reviewed changes

Toni Blaslov added 2 commits December 1, 2022 12:06

Reverting to previous ops value

3b620a1

Simplifying unquantized models logic in test_schema_meta_validation

bfbc562

datumbox approved these changes Dec 1, 2022

View reviewed changes

test/test_extended_models.py Outdated Show resolved Hide resolved

Update test/test_extended_models.py

32de3f3

Co-authored-by: Vasilis Vryniotis <datumbox@users.noreply.github.com>

datumbox added enhancement module: models module: tests labels Dec 1, 2022

Merge branch 'main' into adding-flops-weights-verification

702d8ec

datumbox merged commit 790f1cd into pytorch:main Dec 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding _ops and _weight_size metadata checks to tests #6996

Adding _ops and _weight_size metadata checks to tests #6996

toni057 commented Nov 30, 2022 •

edited by pytorch-bot bot

Loading

datumbox left a comment

datumbox Nov 30, 2022

toni057 Nov 30, 2022

datumbox Nov 30, 2022

datumbox Nov 30, 2022

datumbox Nov 30, 2022

datumbox Nov 30, 2022

datumbox Nov 30, 2022

datumbox Nov 30, 2022

datumbox Nov 30, 2022

datumbox left a comment

datumbox Dec 1, 2022

datumbox left a comment

		return sum(self.flop_counts["Global"].values()) / 1e9


		def get_ops(model: torch.nn.Module, module_name: str, model_name: str, weight: Weights, h=512, w=512):

	if model is None:
	kwargs = {"quantize": True} if module_name == "quantization" else {}
	model = models.get_model(model_name, weights=weight, **kwargs)

		# loading the model and using it for parameter and ops verification
		kwargs = {"quantize": True} if module_name == "quantization" else {}

Adding _ops and _weight_size metadata checks to tests #6996

Adding _ops and _weight_size metadata checks to tests #6996

Conversation

toni057 commented Nov 30, 2022 • edited by pytorch-bot bot Loading

datumbox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datumbox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

datumbox left a comment

Choose a reason for hiding this comment

toni057 commented Nov 30, 2022 •

edited by pytorch-bot bot

Loading