Skip to content

Add more CI testcases for Exynos Backend#18664

Open
Jiseong-oh wants to merge 6 commits into
mainfrom
add_moreTC_quantization
Open

Add more CI testcases for Exynos Backend#18664
Jiseong-oh wants to merge 6 commits into
mainfrom
add_moreTC_quantization

Conversation

@Jiseong-oh

@Jiseong-oh Jiseong-oh commented Apr 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Support necessary stages for E2E test
  • Enable test for float/quant ops and float models

Test plan

cc @SS-JIA @digantdesai @kimishpatel

@pytorch-bot

pytorch-bot Bot commented Apr 2, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18664

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure

As of commit b2c9238 with merge base a9c89f3 (image):

NEW FAILURE - The following job has failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 2, 2026
@github-actions

github-actions Bot commented Apr 2, 2026

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@Jiseong-oh Jiseong-oh force-pushed the add_moreTC_quantization branch from ddd992f to fb39aea Compare April 2, 2026 10:51
@Jiseong-oh Jiseong-oh force-pushed the add_moreTC_quantization branch 2 times, most recently from 8176dcf to b2dc642 Compare April 2, 2026 11:24
@Jiseong-oh Jiseong-oh added partner: samsung For backend delegation, kernels, demo, etc. from the 3rd-party partner, Samsung module: samsung labels Apr 2, 2026
@Jiseong-oh Jiseong-oh force-pushed the add_moreTC_quantization branch 2 times, most recently from ff122a5 to fbdac5c Compare April 7, 2026 02:07
@Jiseong-oh Jiseong-oh force-pushed the add_moreTC_quantization branch 4 times, most recently from 10679bd to 54aab33 Compare April 7, 2026 05:16
Jiseong-oh and others added 2 commits April 8, 2026 05:06
- Support necessary stages for E2E test
- Enable test for float/quant ops and float models

Co-authored-by: chong.checn <chong.chen@samsung.com>
Co-authored-by: jhbb.cha <jhbb.cha@samsung.com>
Co-authored-by: xz.linghu <xz.linghu@samsung.com>
Signed-off-by: jiseong.oh <jiseong.oh@samsung.com>
- this test will be enabled after fixing transformers version issue

Signed-off-by: jiseong.oh <jiseong.oh@samsung.com>
@Jiseong-oh Jiseong-oh force-pushed the add_moreTC_quantization branch from 7a9a6c7 to 27346a7 Compare April 8, 2026 05:09
Jiseong-oh and others added 3 commits April 8, 2026 05:30
- Support necessary stages for E2E test
- Enable test for float/quant ops and float models

Co-authored-by: chong.checn <chong.chen@samsung.com>
Co-authored-by: jhbb.cha <jhbb.cha@samsung.com>
Co-authored-by: xz.linghu <xz.linghu@samsung.com>
Signed-off-by: jiseong.oh <jiseong.oh@samsung.com>
Signed-off-by: jiseong.oh <jiseong.oh@samsung.com>
Internal IR remove "input_type" from Gather op.
This commit removes "input_type" setting and set Gather inputs by

Co-authored-by: Jingya Zhang <jingya.zhang@samsung.com>
Signed-off-by: jiseong.oh <jiseong.oh@samsung.com>
@Jiseong-oh Jiseong-oh force-pushed the add_moreTC_quantization branch from 27346a7 to 12ba2a5 Compare April 8, 2026 05:31
@Jiseong-oh Jiseong-oh requested a review from SS-JIA April 8, 2026 10:13
@SS-JIA

SS-JIA commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

@claude help review this PR

@claude

claude Bot commented Apr 9, 2026

Copy link
Copy Markdown

Claude finished @SS-JIA's task in 4m 51s —— View job


PR Review: Add more CI testcases for Exynos Backend

  • Gather context and understand PR scope
  • Review core passes and builder changes
  • Review test infrastructure (tester, utilities)
  • Review new test cases (ops and models)
  • Review new files (datasets, quant_checkers, mobilebert script)
  • Review CI configuration changes
  • Post comprehensive review feedback

Summary

This PR adds quantized (A8W8) test cases for existing float ops, adds new test utilities (datasets, quant checkers, runtime test runner), introduces a new TransformQuantizedMaskPass, updates several builder ops and passes for quantization support, and adds a MobileBERT fine-tuning QAT example script. The LITECORE version is bumped from v1.0 to v1.1.0.


Issues and Suggestions

1. checker_register decorator does not return the class

backends/samsung/test/utils/quant_checkers.py:70-74

The checker_register decorator registers the class but doesn't return it, which means the decorated classes (ClassifierChecker, SRChecker, SegChecker, W2lChecker) become None at module level. This works because the classes are only accessed through CHECKER_REGISTER dict, but it's a subtle trap — if anyone ever tries to use the class directly (e.g., ClassifierChecker(...)) it will fail.

def checker_register(checker_name: str):
    def _wrapper(cls):
        CHECKER_REGISTER[checker_name] = cls
        # Missing: return cls
    return _wrapper

Fix this →

2. Bare except clauses in mobilebert QAT script

examples/samsung/scripts/mobilebert_finetune_QAT.py:86 and :563

Two bare except: clauses catch all exceptions silently, including KeyboardInterrupt and SystemExit. These should catch specific exceptions (e.g., except (FileNotFoundError, ValueError)) or at minimum use except Exception.

# Line 86
except:  # Should be: except (FileNotFoundError, Exception) as e:
    print(f"Error: the file '{cvs_file_path}' was not avaiable.")

# Line 563
except:  # Should be: except (FileNotFoundError, Exception) as e:
    print(f"Error: the file '{model_path}' was not avaiable.")

3. Hardcoded IP address in TestConfig

backends/samsung/test/utils/utils.py:12

class TestConfig:
    host_ip: str = "111.111.111.111"

The default IP 111.111.111.111 is a real public IP address. Consider using a non-routable placeholder like "0.0.0.0" or "127.0.0.1", or leave it as None and validate at usage time.

4. TransformQuantizedMaskPass.get_mask_mul pattern matching is fragile

backends/samsung/_passes/transform_quantized_mask.py:36-52

The method searches for a node whose target == "attention_mask" (a string match on the node name/target). This is fragile — the name depends on the input parameter naming of the model. If the model uses a different name for the attention mask input, this pass silently does nothing. Consider documenting this requirement or making the target name configurable.

Additionally, the loop at line 43 (while node.target != exir_ops.edge.aten.mul.Tensor) could loop indefinitely if there's a cycle in the graph or an unexpected structure. In practice, FX graphs are DAGs, but a safety counter or max-depth check would be defensive.

5. new_mul variable reassignment in TransformQuantizedMaskPass.transform

backends/samsung/_passes/transform_quantized_mask.py:84-98

The variable new_mul is defined in two consecutive with blocks (lines 84 and 94). While each is scoped to its own block and the logic works, this can be confusing during debugging since the first new_mul is replaced by the second.

6. Missing val meta on newly created nodes in TransformQuantizedMaskPass

backends/samsung/_passes/transform_quantized_mask.py:84-98

The new mul nodes created at lines 84 and 94 set quantize_attrs in their meta but don't set meta["val"]. Other passes (e.g., InsertQDQPass._create_qdq_node at line 48) rely on node.meta["val"]. If these nodes are later processed by such passes, this could cause a KeyError.

7. CI config has commented-out test runner

.github/workflows/pull.yml:1250

#python -m executorch.backends.samsung.test.utils.run_tests --chipset E9955
python -m unittest discover -s backends/samsung/test/models -p "test_*.py"

A commented-out line was added before the existing test command. This suggests the new run_tests.py runner was intended to replace or supplement the existing unittest discover but was left commented out. If this is intentional (e.g., to be enabled later), a TODO comment explaining why would be helpful.

8. print statements for debugging/output in test checkers

backends/samsung/test/utils/quant_checkers.py:100,129,161,199

The checkers use raw print() for output. Consider using logging.info() instead for consistency with the rest of the codebase, or at minimum ensure these don't pollute test output in CI.

9. ClassifierChecker.check() creates tensor with incorrect constructor usage

backends/samsung/test/utils/quant_checkers.py:102

correct = torch.Tensor([0] * max_topk, device="cpu")

torch.Tensor() doesn't accept a device keyword argument in the same way as torch.tensor(). This should be:

correct = torch.zeros(max_topk, device="cpu")

The current code likely creates a tensor with unexpected values or raises an error because torch.Tensor([0, 0, ...], device="cpu") passes device="cpu" as the second positional arg, which is the dtype parameter for torch.Tensor.

Fix this →

10. _annotate_requantize logic change may alter behavior for same-dtype requantization

backends/samsung/_passes/annotate_qparams.py:152-159

The new logic distinguishes between dtype-changing and non-dtype-changing requantization:

  • If dtypes differ → insert requantization node (previous behavior)
  • If dtypes match but other params differ → overwrite node.meta["quantize_attrs"] with the requantize attrs

The second case (line 159) overwrites the original quantize attributes with the last requantize branch's attributes. If a node has multiple output branches where requantization is needed, only the last branch's attributes will be kept. Verify this is the intended behavior when a node feeds multiple consumers with different quantization parameters but the same dtype.

11. AnnotateScalarParametersPass — scale computation for negative scalars

backends/samsung/_passes/annotate_scalar_parameters.py:53-55

if not param_tensor.shape:
    scale = (
        float(param_tensor) if param_tensor > 0 else -float(param_tensor)
    )

This effectively computes abs(param_tensor). Using abs() directly would be clearer:

scale = abs(float(param_tensor))

12. export_utils.py — function type annotation mismatch

backends/samsung/utils/export_utils.py:70

compile_specs: Optional[CompileSpec] = None,

Should be Optional[List[CompileSpec]] since line 76 passes it as a list to EnnPartitioner(compile_specs).


Positive Observations

  • The test structure is well-organized, with consistent _test / _test_a8w8 patterns across all op tests making it easy to follow.
  • The quantization checker infrastructure (CheckerConfig, registry pattern) provides a reasonable framework for validating quantization quality across different model types.
  • The SamsungTester integration with the base Tester harness is clean, properly overriding stages while reusing the common framework.
  • The FuseActivationPass rename from what appears to have been fuse_relu.py is more accurate for the pass's broader scope (handles ReLU6, hardtanh too).

Questions for Authors

  1. The TransformQuantizedMaskPass seems tailored specifically for a MobileBERT attention mask pattern. Is this pass expected to be general-purpose, or is it intentionally model-specific?
  2. The test_mobilebert_finetuning.py test is @unittest.skip-decorated. Is there a tracking issue for fixing the transformer version compatibility mentioned in the comment?
  3. Is the run_tests.py runner intended to eventually replace the unittest discover approach in CI, or serve as a complement for local testing?

@SS-JIA SS-JIA left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stamp to unblock -- but the agent review comments are valid. Would highly recommend fixing them before landing!

@linux-foundation-easycla

Copy link
Copy Markdown

CLA Missing ID

One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via:

Co-authored-by: name <email>

Supported Co-authored-by: formats include:

  1. Anything <id+login@users.noreply.github.com> - it will locate your GitHub user by id part.
  2. Anything <login@users.noreply.github.com> - it will locate your GitHub user by login part.
  3. Anything <public-email> - it will locate your GitHub user by public-email part. Note that this email must be made public on Github.
  4. Anything <other-email> - it will locate your GitHub user by other-email part but only if that email was used before for any other CLA as a main commit author.
  5. login <any-valid-email> - it will locate your GitHub user by login part, note that login part must be at least 3 characters long.

Alternatively, if the co-author should not be included, remove the Co-authored-by: line from the commit message.

Please update your commit message(s) by doing git commit --amend and then git push [--force] and then request re-running CLA check via commenting on this pull request:

/easycla

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: samsung partner: samsung For backend delegation, kernels, demo, etc. from the 3rd-party partner, Samsung

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants