Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ENH: Add variance reconstruction for FDK #632

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

arobert01
Copy link
Collaborator

No description provided.

@arobert01 arobert01 requested a review from SimonRit November 8, 2024 11:03
Copy link
Collaborator

@SimonRit SimonRit left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this overdue PR. A few comments:

  • The command line application is wrong for short scans and displaced detector as you do not square the weights. I would suggest to at least remove them. Are you sure the command line application should be kept? Maybe a Python command line application is sufficient?
  • There is also a test failing: https://my.cdash.org/viewTest.php?onlyfailed&buildid=2717834
  • I would try make the test even faster, maybe 10 projections only? It does not really matter, does it?

@arobert01
Copy link
Collaborator Author

Thanks for your comments.

In the last force-pushed I deleted the application, fixed the failed test and modified the test to be faster.

Jannis Dickmann and others added 3 commits November 27, 2024 15:39
@arobert01
Copy link
Collaborator Author

With the last force-pushed I implemented the idea in the first part of this comment to avoid code repetition.

@SimonRit SimonRit added this to the RTK 2.7 milestone Dec 4, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants