Skip to content

Mark xfail for leaked tracer tests#1997

Merged
fehiepsi merged 8 commits intopyro-ppl:masterfrom
fehiepsi:xfail-leak
Mar 7, 2025
Merged

Mark xfail for leaked tracer tests#1997
fehiepsi merged 8 commits intopyro-ppl:masterfrom
fehiepsi:xfail-leak

Conversation

@fehiepsi
Copy link
Copy Markdown
Member

@fehiepsi fehiepsi commented Mar 6, 2025

This PR adds an entry in github action to track leaked tracer failing tests reported in #1981. I found the behaviors under JAX_CHECK_TRACER_LEAKS is a bit different from the normal behaviors so just want to perform a subset of tests.

Fixes #1981

@fehiepsi
Copy link
Copy Markdown
Member Author

fehiepsi commented Mar 6, 2025

It seems that some issues were fixed so I'll just include the non-passing tests in the PR.

@tillahoffmann
Copy link
Copy Markdown
Collaborator

Looks great! Worth considering a custom marker for this or too much?

# conftest.py
def pytest_runtest_setup(item):
    marker = item.get_closest_marker("xfail_tracer_leak")
    if marker is not None and os.environ.get("JAX_CHECK_TRACER_LEAKS") == "1":
        item.add_marker(pytest.mark.xfail(reason="Expected tracer leak."))


# test files
@pytest.mark.xfail_tracer_leak
def test_...():
    ...

@fehiepsi
Copy link
Copy Markdown
Member Author

fehiepsi commented Mar 7, 2025

Thanks for the suggestion! I prefer using pytest.mark it directly.

There are only 4 issues left (yay!), which I have created separate github issues for each of them.

While debugging, I found that there are many live arrays over the tests so I take this chance to remove all of them (and add a small check to conftest to assure that no live arrays will be created globally).

Copy link
Copy Markdown
Collaborator

@tillahoffmann tillahoffmann left a comment

Choose a reason for hiding this comment

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

Looks great!

@fehiepsi fehiepsi merged commit b766706 into pyro-ppl:master Mar 7, 2025
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.

Tracking leaked tracers.

2 participants