-
Notifications
You must be signed in to change notification settings - Fork 19
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
MultiFab Fixture Cleanup via FabArray::clear
#214
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,47 +85,59 @@ def distmap(boxarr): | |
|
||
|
||
@pytest.fixture(scope="function", params=list(itertools.product([1, 3], [0, 1]))) | ||
def make_mfab(boxarr, distmap, request): | ||
def mfab(boxarr, distmap, request): | ||
"""MultiFab that is either managed or device: | ||
The MultiFab object itself is not a fixture because we want to avoid caching | ||
it between amr.initialize/finalize calls of various tests. | ||
https://github.com/pytest-dev/pytest/discussions/10387 | ||
https://github.com/pytest-dev/pytest/issues/5642#issuecomment-1279612764 | ||
""" | ||
|
||
def create(): | ||
num_components = request.param[0] | ||
num_ghost = request.param[1] | ||
mfab = amr.MultiFab(boxarr, distmap, num_components, num_ghost) | ||
mfab.set_val(0.0, 0, num_components) | ||
return mfab | ||
class MfabContextManager: | ||
def __enter__(self): | ||
num_components = request.param[0] | ||
num_ghost = request.param[1] | ||
self.mfab = amr.MultiFab(boxarr, distmap, num_components, num_ghost) | ||
self.mfab.set_val(0.0, 0, num_components) | ||
return self.mfab | ||
|
||
return create | ||
def __exit__(self, exc_type, exc_value, traceback): | ||
self.mfab.clear() | ||
del self.mfab | ||
|
||
with MfabContextManager() as mfab: | ||
yield mfab | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be simplified? def mfab(boxarr, distmap, request):
num_components = request.param[0]
num_ghost = request.param[1]
try:
mfab = amr.MultiFab(boxarr, distmap, num_components, num_ghost)
mfab.set_val(0.0, 0, num_components)
yield mfab
finally:
mfab.clear()
del mfab There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to my knowledge, that will cache the MultiFab between runs and break out memory pool assumptions... Worth to try again There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interesting. The fixture is marked function scope, so it shouldn't cache a value. Is it pytest that's caching the value? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought so... maybe I misunderstood something when caching kicks in. |
||
|
||
|
||
@pytest.mark.skipif( | ||
amr.Config.gpu_backend != "CUDA", reason="Requires AMReX_GPU_BACKEND=CUDA" | ||
) | ||
@pytest.fixture(scope="function", params=list(itertools.product([1, 3], [0, 1]))) | ||
def make_mfab_device(boxarr, distmap, request): | ||
def mfab_device(boxarr, distmap, request): | ||
"""MultiFab that resides purely on the device: | ||
The MultiFab object itself is not a fixture because we want to avoid caching | ||
it between amr.initialize/finalize calls of various tests. | ||
https://github.com/pytest-dev/pytest/discussions/10387 | ||
https://github.com/pytest-dev/pytest/issues/5642#issuecomment-1279612764 | ||
""" | ||
|
||
def create(): | ||
num_components = request.param[0] | ||
num_ghost = request.param[1] | ||
mfab = amr.MultiFab( | ||
boxarr, | ||
distmap, | ||
num_components, | ||
num_ghost, | ||
amr.MFInfo().set_arena(amr.The_Device_Arena()), | ||
) | ||
mfab.set_val(0.0, 0, num_components) | ||
return mfab | ||
|
||
return create | ||
class MfabDeviceContextManager: | ||
def __enter__(self): | ||
num_components = request.param[0] | ||
num_ghost = request.param[1] | ||
self.mfab = amr.MultiFab( | ||
boxarr, | ||
distmap, | ||
num_components, | ||
num_ghost, | ||
amr.MFInfo().set_arena(amr.The_Device_Arena()), | ||
) | ||
self.mfab.set_val(0.0, 0, num_components) | ||
return self.mfab | ||
|
||
def __exit__(self, exc_type, exc_value, traceback): | ||
self.mfab.clear() | ||
del self.mfab | ||
|
||
with MfabDeviceContextManager() as mfab: | ||
yield mfab |
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.
pylint
would probably create warnings for this.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.
Oh really? Linting still coming in #208 😅
CodeQL did not yet complain, what's to be avoided here?
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.
Pylint will complain about attributes being declared/created in a non-init function.