Skip to content

Conversation

@saulshanabrook
Copy link
Member

This PR closes #241 by adding support for custom models in Python.

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 25, 2025

CodSpeed Instrumentation Performance Report

Merging #357 will degrade performances by 17.08%

Comparing custom-cost-model (9aa04b1) with main (a46b180)

Summary

❌ 1 (👁 1) regression
✅ 6 untouched

Benchmarks breakdown

Benchmark BASE HEAD Change
👁 test_jit[lda] 7.1 s 8.6 s -17.08%

@saulshanabrook saulshanabrook requested a review from Copilot October 2, 2025 20:00
@saulshanabrook
Copy link
Member Author

@codex can you review this

@saulshanabrook saulshanabrook requested review from Copilot and removed request for Copilot October 2, 2025 20:06
@saulshanabrook saulshanabrook marked this pull request as ready for review October 2, 2025 20:06
Copy link
Contributor

Copilot AI left a comment

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 adds support for user-defined cost models in Python by implementing custom extractors that can use Python functions to determine extraction costs. The change enables users to define cost models as Python callable protocols that take an egraph, expression, and children costs, returning the total cost for extraction.

Key changes:

  • Implement custom cost model infrastructure with CostModel protocol and Extractor class
  • Add Value type to bindings for representing egglog values
  • Simplify TermDag implementation using tuple struct pattern

Reviewed Changes

Copilot reviewed 26 out of 27 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/extract.rs New module implementing custom cost models and extractors with Python bindings
src/egraph.rs Add Value type and value conversion methods for different egglog sorts
src/termdag.rs Refactor to use tuple struct pattern and remove redundant From implementations
python/egglog/egraph.py Add cost model protocols, extraction overloads, and default/greedy cost models
python/egglog/declarations.py Add GetCostDecl and ValueDecl expression types for cost model support
Comments suppressed due to low confidence (1)

src/egraph.rs:1

  • Debug print statement should be removed or converted to proper logging.
// Wrapper around EGraph type

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

@saulshanabrook saulshanabrook changed the title User Defined Cost Models in Python Add ability to create custom model and pass them in to extract Oct 2, 2025
@saulshanabrook
Copy link
Member Author

@actions-user changelog

@saulshanabrook saulshanabrook merged commit 18f8ea9 into main Oct 2, 2025
3 of 4 checks passed
@saulshanabrook saulshanabrook deleted the custom-cost-model branch October 2, 2025 23:37
@saulshanabrook
Copy link
Member Author

Thanks @FTRobbin and @oflatt for the idea of how to encode graph extraction in the cost model!

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.

Support custom extraction with cost functions

3 participants