Skip to content

Conversation

@MichaelColonel
Copy link

Implementation of lambdas instead of functor classes in rtkJosephForwardProjectionFilter.
Test example of MIP filter using this new class.

Possible solution of issue #645.

@SimonRit
Copy link
Collaborator

Thanks a lot for doing this. Could you please modify the existing projector instead of creating a new one? That would facilitate the review and this is what we want eventually.
This is a big change so it will take time before we all agree on this...

@MichaelColonel
Copy link
Author

Could you please modify the existing projector instead of creating a new one? That would facilitate the review and this is what we want eventually.

I will do that, but what to do with JosephForwardAttenuatedProjectionImageFilter class?
There are some private members within functors classes, which can't be implemented in lambda functions.

@MichaelColonel
Copy link
Author

I've modified existing classes for JosephForwardProjectionFilter.
Not so sure about JosephForwardAttenuatedProjectionImageFilter though, in any case this PR is ready for a review.

@MichaelColonel MichaelColonel changed the title WIP: lambdas instead of functors in rtkJosephForwardProjectionFilter ENH: Lambda functions instead of functor classes in JosephForwardProjectionFilter Mar 4, 2025
…ardProjectionFilter

Lambdas were added to the classes:
  JosephForwardProjectionImageFilter,
  JosephForwardAttenuatedProjectionImageFilter,
  MaximumIntensityProjectionImageFilter.

Python wrappers for classes above have been modified.

Similar lambda functions can be added to back projection classes.
Copy link
Collaborator

@arobert01 arobert01 left a comment

Choose a reason for hiding this comment

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

Thank you for the great work! It looks really good to me. I made some comments, mostly related to style.
Regarding JosephAttenuatedForwardProjector, based on the tests I did, everything seems to work well, and I don’t see a better way to do it. Maybe @SimonRit will have additional comments before merging.
If there are no further comments, we can also implement these changes in the Joseph-based back projectors.
Thanks again!

Comment on lines 70 to 71
*
* \author Simon Rit
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would remove the author part of the comments.

Copy link
Author

Choose a reason for hiding this comment

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

Ok.

using InputPixelType = typename TInputImage::PixelType;
using OutputPixelType = typename TOutputImage::PixelType;
using CoordinateType = double;
using WeightCoordinateType = typename itk::PixelTraits<InputPixelType>::ValueType;
Copy link
Collaborator

Choose a reason for hiding this comment

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

WeightCoordinateType doesn't seem to be used in this case

Copy link
Author

Choose a reason for hiding this comment

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

It will be removed.


std::cout << "\n\n****** Compare two resulted images ******" << std::endl;
CheckImageQuality<OutputImageType>(jfp->GetOutput(), mipfp->GetOutput(), 0.001, 0.000001, 1.E+19);
std::cout << "\n\nImages are OK! " << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe I would keep the same "Test PASSED! " sentence for consistency ?

Copy link
Author

Choose a reason for hiding this comment

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

I will fix that.

/** \brief Function to multiply the interpolation weights with the projected
* volume values and attenuation map.
*
* \author Antoine Robert
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove author section in the comments

Copy link
Author

Choose a reason for hiding this comment

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

Ok.

@SimonRit
Copy link
Collaborator

Thank you for your excellent contribution @MichaelColonel and your review @arobert01. This is very interesting work but I will include it in the next RTK major release. I am currently working on a minor release and will merge this one afterwards. Note that #629 will probably require additional work for this PR but I'll handle it.

@SimonRit SimonRit added this to the RTK 3.0 milestone Mar 24, 2025
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.

3 participants