Merge Ricsi's second branch with first branch. #3

richardsipos · 2025-12-18T18:28:35Z

Merge Task 6 solution with Task 4 solution.

… and run.

richardsipos · 2025-12-18T18:29:41Z

Merge completed.

Copilot

Pull request overview

This PR merges Task 6 and Task 4 solutions, integrating Hungarian assignment algorithm support and Hydra multi-run configuration capabilities into the formation environment codebase.

Key Changes:

Added scipy dependency and implemented Hungarian assignment algorithm testing
Configured Hydra multi-run sweeps for comparing greedy vs Hungarian assignment methods across multiple seeds
Updated Docker configuration to improve build reliability with Python 3.11 and optimized layer ordering

Reviewed changes

Copilot reviewed 5 out of 7 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
test/test_env.py	Added two new test methods: one for SDF circle distance calculations and another comparing greedy vs Hungarian assignment behavior
requirements.txt	Added scipy dependency required for Hungarian algorithm implementation
pytest.ini	Configured pytest to exclude large output directories (wandb, outputs, multirun) to prevent OSErrors on Windows
docker/Dockerfile	Downgraded to Python 3.11, reorganized COPY commands to optimize Docker layer caching by copying requirements.txt before the full source
configs/experiment/sweep.yaml	Added Hydra multi-run configuration to sweep over seeds (0,1,2) and assignment methods (greedy, hungarian)
.gitignore	Added multirun/ directory to ignore list for Hydra sweep outputs
.dockerignore	Created file to exclude large folders (wandb, outputs, multirun) from Docker build context

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-18T18:35:31Z

test/test_env.py

+            ],
+            dim=0,
+        ).to(self.device)
+        env_hungarian._FormationEnv__update_assignments()


Accessing a private method using name mangling (_FormationEnv__update_assignments) bypasses encapsulation and makes the test tightly coupled to the implementation. Consider making this method protected (single underscore) or providing a public API for testing purposes, which would make tests more maintainable if the class is renamed or refactored.

Copilot · 2025-12-18T18:35:32Z

test/test_env.py

        result_td = env.step(action_td)
        self.assertIsNotNone(result_td)
+
+    def test_sdf_circle_known_points(self):


The test method name 'test_sdf_circle_known_points' uses the abbreviation 'sdf' which may not be immediately clear. Consider using a more descriptive name like 'test_signed_distance_function_circle_known_points' or 'test_circle_signed_distance_calculation' to improve test readability.

Suggested change

def test_sdf_circle_known_points(self):

def test_signed_distance_function_circle_known_points(self):

Copilot · 2025-12-18T18:35:32Z

docker/Dockerfile

@@ -1,5 +1,8 @@
-FROM python:3.12-slim
+


The Dockerfile now includes blank lines at the beginning which are unnecessary. Consider removing the empty line at line 1 to keep the Dockerfile clean and concise.

Copilot · 2025-12-18T18:35:32Z

docker/Dockerfile

-FROM python:3.12-slim
+
+FROM python:3.11-slim
+


Consider removing the blank line after FROM statement to keep the Dockerfile more concise and follow conventional Docker formatting practices.

Suggested change

Copilot · 2025-12-18T18:35:33Z

test/test_env.py

+    def test_assignment_hungarian_is_one_to_one(self):
+        """Hungarian assignment should make one-to-one assignments. Greedy will prefer closer targets."""
+
+        # Greedy assignment
+        cfg_greedy = OmegaConf.merge(self.cfg, {"env": {"num_agents": 2, "assignment_method": "greedy"}})
+        env_greedy = FormationEnv(cfg=cfg_greedy, device=self.device)
+        # Both agents close to the same target point
+        target0 = env_greedy.shape_boundary_points[0].detach().cpu()
+        env_greedy.agent_positions = torch.stack(
+            [
+                target0 + torch.tensor([0.01, 0.0]),
+                target0 + torch.tensor([0.02, 0.0]),
+            ],
+            dim=0,
+        ).to(self.device)
+        env_greedy._FormationEnv__update_assignments()
+        unique_greedy = torch.unique(env_greedy.assigned_target_positions.cpu(), dim=0).shape[0]
+
+        # Hungarian assignment
+        cfg_hungarian = OmegaConf.merge(self.cfg, {"env": {"num_agents": 2, "assignment_method": "hungarian"}})
+        env_hungarian = FormationEnv(cfg=cfg_hungarian, device=self.device)
+        # Both agents close to the same target point
+        target0_h = env_hungarian.shape_boundary_points[0].detach().cpu()
+        env_hungarian.agent_positions = torch.stack(
+            [
+                target0_h + torch.tensor([0.01, 0.0]),
+                target0_h + torch.tensor([0.02, 0.0]),
+            ],
+            dim=0,
+        ).to(self.device)
+        env_hungarian._FormationEnv__update_assignments()
+        unique_hungarian = torch.unique(env_hungarian.assigned_target_positions.cpu(), dim=0).shape[0]
+


The test contains significant code duplication between the greedy and Hungarian assignment test sections. Consider extracting a helper method that accepts the assignment method as a parameter and returns the unique count, which would reduce duplication and improve maintainability.

Suggested change

def test_assignment_hungarian_is_one_to_one(self):

"""Hungarian assignment should make one-to-one assignments. Greedy will prefer closer targets."""

# Greedy assignment

cfg_greedy = OmegaConf.merge(self.cfg, {"env": {"num_agents": 2, "assignment_method": "greedy"}})

env_greedy = FormationEnv(cfg=cfg_greedy, device=self.device)

# Both agents close to the same target point

target0 = env_greedy.shape_boundary_points[0].detach().cpu()

env_greedy.agent_positions = torch.stack(

[

target0 + torch.tensor([0.01, 0.0]),

target0 + torch.tensor([0.02, 0.0]),

],

dim=0,

).to(self.device)

env_greedy._FormationEnv__update_assignments()

unique_greedy = torch.unique(env_greedy.assigned_target_positions.cpu(), dim=0).shape[0]

# Hungarian assignment

cfg_hungarian = OmegaConf.merge(self.cfg, {"env": {"num_agents": 2, "assignment_method": "hungarian"}})

env_hungarian = FormationEnv(cfg=cfg_hungarian, device=self.device)

# Both agents close to the same target point

target0_h = env_hungarian.shape_boundary_points[0].detach().cpu()

env_hungarian.agent_positions = torch.stack(

[

target0_h + torch.tensor([0.01, 0.0]),

target0_h + torch.tensor([0.02, 0.0]),

],

dim=0,

).to(self.device)

env_hungarian._FormationEnv__update_assignments()

unique_hungarian = torch.unique(env_hungarian.assigned_target_positions.cpu(), dim=0).shape[0]

def _get_unique_assigned_targets(self, assignment_method: str) -> int:

"""Helper to compute the number of unique assigned target positions for a given method."""

cfg = OmegaConf.merge(

self.cfg,

{"env": {"num_agents": 2, "assignment_method": assignment_method}},

)

env = FormationEnv(cfg=cfg, device=self.device)

# Both agents close to the same target point

target0 = env.shape_boundary_points[0].detach().cpu()

env.agent_positions = torch.stack(

[

target0 + torch.tensor([0.01, 0.0]),

target0 + torch.tensor([0.02, 0.0]),

],

dim=0,

).to(self.device)

env._FormationEnv__update_assignments()

return torch.unique(env.assigned_target_positions.cpu(), dim=0).shape[0]

def test_assignment_hungarian_is_one_to_one(self):

"""Hungarian assignment should make one-to-one assignments. Greedy will prefer closer targets."""

unique_greedy = self._get_unique_assigned_targets("greedy")

unique_hungarian = self._get_unique_assigned_targets("hungarian")

Copilot · 2025-12-18T18:35:33Z

test/test_env.py

+            ],
+            dim=0,
+        ).to(self.device)
+        env_greedy._FormationEnv__update_assignments()


Accessing a private method using name mangling (_FormationEnv__update_assignments) bypasses encapsulation and makes the test tightly coupled to the implementation. Consider making this method protected (single underscore) or providing a public API for testing purposes, which would make tests more maintainable if the class is renamed or refactored.

Copilot · 2025-12-18T18:35:33Z

docker/Dockerfile

+
 WORKDIR /app
+COPY requirements.txt /app/requirements.txt
+RUN pip install -r /app/requirements.txt


The RUN pip install -r /app/requirements.txt step installs third-party dependencies from PyPI without any version pinning or integrity guarantees, so each build may pull a different mutable release, including potentially compromised or typosquatted packages. Since these packages execute code at install/import time inside the build/runtime environment, an attacker controlling a dependency version could gain code execution or exfiltrate secrets. Pin all dependencies in requirements.txt to fixed versions (and ideally verify hashes) so this Docker build step only installs vetted artifacts.

richardsipos added 4 commits December 18, 2025 00:30

Add multirun sweep configuration.

3f14f01

Fix issue of too large folders and not found main during docker build…

8870b24

… and run.

Add missing requirement. Finish docker fix.

86ef7fb

Fix windows unit test issue. Add SDF and Assignments Unit test.

121954a

Copilot AI review requested due to automatic review settings December 18, 2025 18:28

Copilot started reviewing on behalf of richardsipos December 18, 2025 18:29 View session

richardsipos merged commit 4b0f66a into ricsi/branch1 Dec 18, 2025
8 of 10 checks passed

richardsipos deleted the ricsi/branch2 branch December 18, 2025 18:29

Copilot AI reviewed Dec 18, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merge Ricsi's second branch with first branch. #3

Merge Ricsi's second branch with first branch. #3

Uh oh!

richardsipos commented Dec 18, 2025

Uh oh!

Uh oh!

richardsipos commented Dec 18, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	def test_sdf_circle_known_points(self):
	def test_signed_distance_function_circle_known_points(self):

Merge Ricsi's second branch with first branch. #3

Merge Ricsi's second branch with first branch. #3

Uh oh!

Conversation

richardsipos commented Dec 18, 2025

Uh oh!

Uh oh!

richardsipos commented Dec 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants