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.

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

Fix of BlocksOnCylindrical projector tests #1342

Conversation

markus-jehl
Copy link
Contributor

Changes in this pull request

Added check for view mashing for generic proj data and fixed the blocks on cylindrical projection teests. Muted excessive logging in forward projector by default (was undone in TOF merge).

Testing performed

Ran blocks on cylindrical projection test in debug.

Related issues

fixes #1335

Checklist before requesting a review

  • I have performed a self-review of my code
  • [] I have added docstrings/doxygen in line with the guidance in the developer guide
  • [] I have implemented unit tests that cover any new or modified functionality (if applicable)
  • The code builds and runs on my machine
  • [] documentation/release_XXX.md has been updated with any functionality change (if applicable)

…ks on cylindrical projection teests. Muted excessive logging in forward projector by default (was undone in TOF merge).
@robbietuk
Copy link
Collaborator

In the GHA config, can test_blocks_on_cylindrical_projectors be removed from the exclude list when in debug?
#1335 (comment)

@KrisThielemans
Copy link
Collaborator

good idea, but only if it doesn't take too long to run...

@markus-jehl
Copy link
Contributor Author

They take roughly 10 minutes on my machine in debug. Is that too long or acceptable?

@KrisThielemans
Copy link
Collaborator

I had a quick look at the Actions. ATM, ctest takes: Linux-Release 3min , Linux-Debug 30 mins, MacxOS Debug 60 mins. I'm assuming @markus-jehl machine is quite a lot more powerful than what we get on GHA, so that seems to be problematic. We'd have to wait a fairly long time for every commit, might get into run-time thresholds, and in any case it isn't that great for energy usage.

Not sure if it's easy to reduce the scanner num_detectors_per_ring without further code adjustments, but if it is, I'd prefer that.

…st and added its debug build to the github workflow.
@markus-jehl
Copy link
Contributor Author

The downsampled scanner takes 1.5 minutes on my machine - that could be a nice middle ground.

@KrisThielemans
Copy link
Collaborator

ok. 350s for this test (total time for ctest 32min) in Linux-debug, 1053s (total time for ctest 1u15) for MacOS-Debug. This test is of the same order as the other long tests.

There would be a way to disable many array-access tests in debug mode, which would speed things up, but that'll be for another day.

thanks!

@KrisThielemans KrisThielemans merged commit a129c2f into UCL:master Jan 26, 2024
7 checks passed
@markus-jehl markus-jehl deleted the issue/1335-test-blocks-on-cylindrical-projectors-fails branch January 29, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_blocks_on_cylindrical_projectors fails when compiled in debug
3 participants