Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add test to reproduce SimpleITK issue #2220 #5070

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add test to reproduce SimpleITK issue #2220 #5070

wants to merge 1 commit into from

Conversation

blowekamp
Copy link
Member

Incorrect results are being computed on windows with the Compose filter ( and maybe other filters) and VectorImages of sizes >4GB.

See: SimpleITK/SimpleITK#2220 for details

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

@blowekamp blowekamp requested a review from dzenanz December 14, 2024 20:31
@github-actions github-actions bot added type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Filtering Issues affecting the Filtering module labels Dec 14, 2024
@blowekamp blowekamp mentioned this pull request Dec 15, 2024
Merged
7 tasks
@blowekamp
Copy link
Member Author

The fix for this issue was addressed in #5072. Would it be work the additional CI time to add this memory intensive test?

@dzenanz
Copy link
Member

dzenanz commented Jan 3, 2025

A dozen more seconds spent on testing is not an issue. But if high memory usage of this test interferes with other tests, that might be an issue.

@blowekamp
ENH: add test to reproduce SimpleITK issue InsightSoftwareConsortium#…
b40eb7c 
…2220

Incorrect results are being computed on windows with the Compose
filter (and other filters) and VectorImages of sizes >4GB.

Bug was addressed in PR InsightSoftwareConsortium#5071.
@blowekamp blowekamp changed the title WIP: add test to reproduce SimpleITK issue #2220 add test to reproduce SimpleITK issue #2220 Jan 6, 2025
@blowekamp blowekamp marked this pull request as ready for review January 6, 2025 14:34
@blowekamp
Copy link
Member Author

A dozen more seconds spent on testing is not an issue. But if high memory usage of this test interferes with other tests, that might be an issue.

The CTest properties should ensure that the high memory usage does not conflict with other high memory usage tests. The test takes 21 seconds in debug mode on my laptop.

@dzenanz
Copy link
Member

dzenanz commented Jan 6, 2025

That is low enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dzenanz dzenanz dzenanz approved these changes

@thewtex thewtex Awaiting requested review from thewtex

@hjmjohnson hjmjohnson Awaiting requested review from hjmjohnson

Assignees
No one assigned
Labels
area:Filtering Issues affecting the Filtering module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@blowekamp @dzenanz