Skip to content

Add more CI testcases for Exynos Backend#18664

Open
Jiseong-oh wants to merge 5 commits intomainfrom
add_moreTC_quantization
Open

Add more CI testcases for Exynos Backend#18664
Jiseong-oh wants to merge 5 commits intomainfrom
add_moreTC_quantization

Conversation

@Jiseong-oh
Copy link
Copy Markdown
Collaborator

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

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
Copy link
Copy Markdown

pytorch-bot bot commented Apr 2, 2026

🔗 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.

❌ 3 New Failures, 4 Unrelated Failures

As of commit 12ba2a5 with merge base b5ae0b9 (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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
Copy link
Copy Markdown

github-actions bot commented Apr 2, 2026

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
Copy link
Copy Markdown
Contributor

SS-JIA commented Apr 9, 2026

@claude help review this PR

@claude
Copy link
Copy Markdown

claude bot commented Apr 9, 2026

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?

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