Skip to content

Refactor: Extract common injection logic into strategy pattern#59

Draft
Copilot wants to merge 6 commits intodevfrom
copilot/refactor-run-injection-function
Draft

Refactor: Extract common injection logic into strategy pattern#59
Copilot wants to merge 6 commits intodevfrom
copilot/refactor-run-injection-function

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 10, 2025

The _run_injector_impl methods in ExhaustiveSEUInjector and StochasticSEUInjector were 85+ lines each with substantial duplication. Only the index selection strategy differed between implementations.

Changes

Extracted 6 helper methods to BaseInjector:

  • _iterate_layers() - Layer filtering and iteration
  • _prepare_tensor_for_injection() - Tensor cloning and CPU conversion
  • _inject_and_evaluate() - Bitflip, evaluation, restoration cycle
  • _record_injection_result() - Result dictionary population
  • _initialize_results() - Result structure creation
  • _get_injection_indices() - Abstract strategy method

Simplified implementations:

  • ExhaustiveSEUInjector._run_injector_impl: 85 → 35 lines
  • StochasticSEUInjector._run_injector_impl: 85 → 32 lines
  • Each now implements only _get_injection_indices() for strategy

Added test coverage:

  • 7 unit tests for extracted helpers
  • 100% coverage for all core injector classes

Impact

New injector types require implementing only the selection strategy:

class AdaptiveSEUInjector(BaseInjector):
    def _get_injection_indices(self, tensor_shape, **kwargs):
        # Custom selection logic
        adaptive_prob = self._compute_adaptive_probability(tensor_shape, **kwargs)
        mask = np.random.random(tensor_shape) < adaptive_prob
        return np.argwhere(mask)
    
    # _run_injector_impl inherited, no duplication needed

All existing tests pass. No API changes.

Original prompt

This section details on the original issue you should resolve

<issue_title>[FEATURE] run_injection function is too large</issue_title>
<issue_description>### Feature Summary

This function should be broken down into smaller functions

Feature Category

API Enhancement

Problem Statement

Make the development easier

Proposed Solution

No response

Proposed API (if applicable)

Priority Level

None

Research Impact

No response

Alternatives Considered

No response

Implementation Considerations

  • This feature requires new dependencies
  • This feature affects performance-critical paths
  • This feature requires GPU/CUDA support
  • This feature needs extensive testing
  • This feature affects the public API
  • I would be interested in implementing this feature

Additional Context

No response

Pre-submission Checklist

  • I have searched existing issues for similar requests
  • I have provided sufficient detail about the use case
  • I have considered the impact on existing users
  • I have thought about backward compatibility</issue_description>

<agent_instructions>Can you propose how to modularise this core function. Be forward thinking for future types of injectors so that the API does not need adjusting </agent_instructions>

Comments on the Issue (you are @copilot in this section)

@wd7512 Looks like this should be broken down into smaller issues

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits December 10, 2025 09:25
… in BaseInjector

Co-authored-by: Will-D-AER <197624355+Will-D-AER@users.noreply.github.com>
Co-authored-by: Will-D-AER <197624355+Will-D-AER@users.noreply.github.com>
Co-authored-by: Will-D-AER <197624355+Will-D-AER@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor run_injection function into smaller functions Refactor: Extract common injection logic into strategy pattern Dec 10, 2025
Copilot AI requested a review from Will-D-AER December 10, 2025 09:33
@wd7512
Copy link
Copy Markdown
Owner

wd7512 commented Dec 25, 2025

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is an excellent refactoring that extracts common injection logic from ExhaustiveSEUInjector and StochasticSEUInjector into a BaseInjector class, effectively applying the Strategy design pattern. This change significantly reduces code duplication, improves maintainability, and makes the system more extensible for future injector types. The simplification of the _run_injector_impl methods is clean, and the addition of comprehensive unit tests for the new helper methods ensures the correctness of this refactoring. I have one minor suggestion to further improve performance.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants