Skip to content

MP cleanup and partial refactoring#1391

Merged
irenaby merged 12 commits intomainfrom
mp_refactor3
Mar 23, 2025
Merged

MP cleanup and partial refactoring#1391
irenaby merged 12 commits intomainfrom
mp_refactor3

Conversation

@irenaby
Copy link
Copy Markdown
Contributor

@irenaby irenaby commented Mar 16, 2025

Pull Request Description:

  • Simplify resource utilization constraints construction - compute total weights/bops directly instead of per node and aggregating at a later stage. Use RUCalculator to compute all targets at once and use the results directly. Remove separate computation for non-configurable nodes. Remove all intermediate methods per target and the double api for conf/non-conf which are no longer needed. Remove special hacky handling of total ru metric, which is now obtained automatically by RUCalculator along with the rest.
  • Convert LP solver module functions into class, remove dependency on MPSearchManager, move all code that is not related to LP to MPSearchManager.
  • Move more of MP search responsibility to MPSearchManager and make it the entry point for the search. The manager precomputes all metrics (sensitivity and ru) and uses LPSolver only for building and solving LP. Move virtual graph creation and restoring final config to MPSearchManager, and only create virtual graph for BOPS if both weights and activations are configurable (previously was created in any case of BOPS).

Checklist before requesting a review:

  • I set the appropriate labels on the pull request.
  • I have added/updated the release note draft (if necessary).
  • I have updated the documentation to reflect my changes (if necessary).
  • All function and files are well documented.
  • All function and classes have type hints.
  • There is a licenses in all file.
  • The function and variable names are informative.
  • I have checked for code duplications.
  • I have added new unittest (if necessary).

f"following targets: {unsatisfiable_targets}")
return rel_target_ru

def _build_sensitivity_mapping(self, eps: float = EPS) -> Dict[int, Dict[int, float]]:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved from lp file, no change in implementation

in self.layer_to_indicator_vars_mapping.values()]
).flatten()

return config.tolist()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Config reconstruction was moved to MP manager

return config.tolist()

@staticmethod
def _init_problem_vars(layer_to_metrics_mapping: Dict[int, Dict[int, float]]) -> Tuple[
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No functional changes in this method


return layer_to_indicator_vars_mapping, layer_to_objective_vars_mapping

def _formalize_problem(self) -> LpProblem:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No functional changes in this method


return lp_problem

def _add_ru_constraints(self, lp_problem: LpProblem):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This method was modified.

@@ -57,144 +57,122 @@ def reconstruct_config_from_virtual_graph(self,


class MockMixedPrecisionSearchManager:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed to pass. Will be replaced by new tests.

@irenaby irenaby requested review from elad-c and ofirgo and removed request for ofirgo March 17, 2025 08:00
@irenaby irenaby marked this pull request as ready for review March 17, 2025 08:00
@ofirgo ofirgo removed their request for review March 17, 2025 08:32
Copy link
Copy Markdown
Contributor

@elad-c elad-c left a comment

Choose a reason for hiding this comment

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

In general for tests: write a comment on each stating what that test tests.

@irenaby
Copy link
Copy Markdown
Contributor Author

irenaby commented Mar 23, 2025

In general for tests: write a comment on each stating what that test tests.

In general I agree, but no new tests were added, so I'm not sure what exactly you refer to.

@irenaby irenaby merged commit 20c321c into main Mar 23, 2025
36 checks passed
@ofirgo ofirgo deleted the mp_refactor3 branch July 2, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants