Skip to content

Add a simple contract system to the time travel debugger. #1565

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

femtomc
Copy link
Collaborator

@femtomc femtomc commented Mar 15, 2025

No description provided.

@femtomc femtomc requested a review from fzaiser March 15, 2025 03:12
Copy link

gitstream-cm bot commented Mar 15, 2025

🥷 Code experts: sritchie

sritchie, sharlaon have most 👩‍💻 activity in the files.

See details

src/genjax/_src/core/interpreters/time_travel.py

Activity based on git-commit:

sritchie sharlaon
MAR
FEB 2 additions & 2 deletions
JAN
DEC 3 additions & 3 deletions
NOV 1 additions & 1 deletions
OCT 1 additions & 1 deletions 6 additions & 8 deletions

Knowledge based on git-blame:

src/genjax/time_travel.py

Activity based on git-commit:

sritchie sharlaon
MAR
FEB
JAN
DEC
NOV
OCT

Knowledge based on git-blame:

To learn more about /:\ gitStream - Visit our Docs

Copy link
Contributor

@fzaiser fzaiser left a comment

Choose a reason for hiding this comment

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

This is a cool addition! The code looks reasonable to me, just some more docstrings would be nice. Also adding a test could be good as it would also serve as documentation of the interface.



@Pytree.dataclass
class Breakpoint(Generic[R, S], Pytree):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a docstring?

@@ -77,21 +134,15 @@ def _cont_prim_call(brk_pt, *args):
return initial_style_bind(record_p)(_cont_prim_call)(self, *args)


def rec(
def brk(
callable: Callable[..., R],
debug_tag: str | None = None,
):
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a docstring? For instance, it was unclear to me what callable is used for. First I thought it was the continuation for after the breakpoint, but Breakpoint.handle is already given a continuation.

@femtomc
Copy link
Collaborator Author

femtomc commented Mar 17, 2025

@fzaiser thanks for taking a look! I’ll add a bunch of docstrings and tests and ask for a quick re-review sometime this week / next

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