Skip to content

Fix DAG extract bug - #388

Open
saulshanabrook wants to merge 2 commits into
mainfrom
fix-bug-extract
Open

Fix DAG extract bug#388
saulshanabrook wants to merge 2 commits into
mainfrom
fix-bug-extract

Conversation

@saulshanabrook

@saulshanabrooksaulshanabrook commented Dec 10, 2025

Copy link
Copy Markdown
Member

Fixes#387 by updating egglog version. Also tested this on numba/sealir#15 usage of the dag cost extractor and it seems to work

CopilotAI review requested due to automatic review settings December 10, 2025 02:18
@saulshanabrooksaulshanabrook changed the title Fix bug extractFix DAG extract bugDec 10, 2025

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes issue #387 by updating the egglog dependency from the "fix-fn-bug" branch to the "dag-extract" branch, which includes a fix for extraction with DAG-based cost models.

Key Changes:

  • Updated all egglog-related dependencies to use the "dag-extract" branch instead of "fix-fn-bug"
  • Added a comprehensive test case that verifies the extraction bug fix with a custom DAG-based cost model

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

FileDescription
python/tests/test_extract_bug.pyNew test file implementing a DAG-based cost model and testing extraction behavior to verify the bug fix
Cargo.tomlUpdated 10 egglog dependency references from "fix-fn-bug" to "dag-extract" branch
Cargo.lockUpdated dependency lock file with new commit hashes corresponding to the "dag-extract" branch

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

Comment threadpython/tests/test_extract_bug.py
Comment threadpython/tests/test_extract_bug.py
Comment threadpython/tests/test_extract_bug.py
@codspeed-hq

Copy link
Copy Markdown

CodSpeed Performance Report

Merging #388 will not alter performance

Comparing fix-bug-extract (aab8e15) with main (6a892ec)

Summary

✅ 14 untouched

Sign up for freeto 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.

Rust panic when extracting using greedy_dag_cost_model

2 participants

@saulshanabrook