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: Pad BSpline design matrix #319

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

effigies
Copy link
Member

@effigies effigies commented Dec 9, 2022

If the bounding box of the target image goes outside the bounding box of the B-spline coefficient image, scipy will refuse to construct a design matrix. This is because the coordinates x (voxel indices translated into non-integral indices in the coefficient image's voxel space) may go below 0 or above the number of knots.

This PR aims to work around this limitation without changing the logic on the grounds that we need the best approximation of the field at all locations within the target image. At the edges, the field is likely to get masked out anyway. This assumes that the fieldmap and target image are in register and the differences in their affines reflect real differences in FoV.

We have a default padding of 4 knots in each direction, which gets reduced to 2 by BSpline.design_matrix. This adds in a padding based on the min/max values of x. The padding is added to t and then removed from the design matrix to ensure that the weights still correspond to the coefficients stored in the coefficient images.

@effigies effigies requested a review from oesteban December 9, 2022 13:30
@effigies
Copy link
Member Author

effigies commented Dec 9, 2022

@mgxd The result isn't great, but at least it runs:

image

@effigies
Copy link
Member Author

effigies commented Dec 9, 2022

I think the issue may be the translation:

image
image

I'm going to see what happens if I simply shift t by x.min().

@effigies
Copy link
Member Author

effigies commented Dec 9, 2022

I'm going to see what happens if I simply shift t by x.min().

That made it worse. Is this at least back to where it was previously?

@mgxd
Copy link
Contributor

mgxd commented Dec 9, 2022

Yup - I'm okay with merging this so we can test with #317

@effigies
Copy link
Member Author

effigies commented Dec 9, 2022

I tested with both to get the above. But let's merge this at least.

@effigies effigies merged commit b11551e into nipreps:master Dec 9, 2022
@effigies effigies deleted the fix/oblique_padding branch December 9, 2022 17:01
@effigies effigies mentioned this pull request Dec 9, 2022
7 tasks
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.

2 participants