Skip to content

Fix get_problem_ids to respect instance_ids parameter in SWE environments #179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 22, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 10, 2025

The get_problem_ids method in SWESmithEnv and SWEBenchEnv was not respecting the instance_ids parameter when returning all available problems. This caused issues when users tried to filter tasks using the CLI parameter -p base.env_kwargs.instance_ids=["task1", "task2"].

Problem

When running scripts/run.py with instance filtering:

python scripts/run.py config.yaml -p base.env_kwargs.instance_ids='["task1", "task2"]'

The environment would correctly load only the specified tasks in the dataset, but get_problem_ids("all") would ignore the instance_ids parameter and return all available problems instead of just the filtered ones. This caused the script to attempt running all problems, leading to errors when accessing tasks that weren't loaded.

Solution

Modified the get_problem_ids method in both environments to filter results by instance_ids when provided:

def get_problem_ids(self, split_or_problem_id):
    if split_or_problem_id == "all":
        all_tasks = sorted(k for k in self.dataset.keys() if k not in self.excluded_ids)
        # If instance_ids is provided, filter to only include those tasks
        if self.instance_ids is not None:
            all_tasks = [task for task in all_tasks if task in self.instance_ids]
        return all_tasks
    # ... rest of method unchanged

Changes

  • SWESmithEnv: Added instance_ids filtering while preserving exclusion logic
  • SWEBenchEnv: Added instance_ids filtering for consistent behavior
  • Tests: Added comprehensive test cases for both environments covering edge cases
  • Backward compatibility: When instance_ids=None, behavior remains unchanged

Testing

The fix handles various scenarios correctly:

  • Normal filtering: instance_ids=["task1", "task2"] returns only those tasks
  • Non-existent tasks: instance_ids=["task1", "missing"] returns ["task1"]
  • Excluded tasks: Respects exclusion rules in SWESmithEnv
  • Backward compatibility: instance_ids=None returns all available tasks

Fixes #178.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] merge "base.problems" with "env_kwargs.instance_ids" Fix get_problem_ids to respect instance_ids parameter in SWE environments Jul 10, 2025
@Copilot Copilot AI requested a review from sordonia July 10, 2025 13:28
Copilot finished work on behalf of sordonia July 10, 2025 13:28
@MarcCote
Copy link
Collaborator

@copilot we want to remove instance_ids completley and only rely on "problems" to know which docker images to download.

Copy link
Contributor Author

Copilot AI commented Jul 10, 2025

@copilot we want to remove instance_ids completley and only rely on "problems" to know which docker images to download.

I've removed the instance_ids parameter completely from both SWEBenchEnv and SWESmithEnv and replaced it with a problems parameter. This change ensures that docker images are downloaded only for the specific problems that will be run, rather than being filtered at the environment level through instance_ids. The change is in commit 3daffaa.

@Copilot Copilot AI requested a review from MarcCote July 10, 2025 18:28
Copilot finished work on behalf of MarcCote July 10, 2025 18:28
Copy link
Collaborator

@MarcCote MarcCote left a comment

Choose a reason for hiding this comment

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

I pushed changes that implement the fix properly. @copilot

@MarcCote MarcCote marked this pull request as ready for review July 21, 2025 18:08
Copy link
Collaborator

@matheper matheper left a comment

Choose a reason for hiding this comment

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

LGTM

@MarcCote MarcCote merged commit 27efbe8 into main Jul 22, 2025
6 checks passed
@MarcCote MarcCote deleted the copilot/fix-178 branch July 22, 2025 12:48
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.

merge "base.problems" with "env_kwargs.instance_ids"
4 participants