-
Notifications
You must be signed in to change notification settings - Fork 30
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
Remove pytest cache clearing #375
Conversation
cpjordan
commented
Oct 17, 2024
•
edited
Loading
edited
- This was updated in Firedrake PR #3730
- This was updated in Firedrake commit #3730
Firedrake PR #3370 and PyOP2 PR #724 updated the caching mechanisms which caused the hooks in test/conftest.py and test_adjoint/conftest.py to fail. These hooks were originally introduced in PR #2 to reduce memory usage and also prevent errors caused by inter-test dependencies. With the recent caching updates in Firedrake and PyOP2, these issues should be resolved, so the cache-clearing hooks are no longer necessary, or at least until further issues arise. |
This looks good wrt tsfc/pyop2 cache but I would leave the clearing of the adjoint tape in place as it is. If you don't clear the tape between tests that use the adjoint then the model annotated in each tests just gets appended. This is a massive memory hole as you end up having the annotated forward model of each test in memory simultaneously. It potentially also slows down things, as a replay/backward pass may (depending on how clever pyadjoint is being) end up going through the entire tape including all operations of previous tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good to me
Rerun CI in firedrake-vanilla/2024-10 Roll back Thetis to 12 Nov (merge of #375)