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

feat: Allowed option to reflectively pad projection DFT for reduced high res artifacts #8

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

Conversation

jdickerson95
Copy link

Added reflective padding of central FFT slice in project.py to reduce artefacts close to Nyquist.
The rest of the changes were just to get it past the pre-commit.

@alisterburt
Copy link
Member

hey! cool PR but I think this padding is already handled here in torch_image_lerp (a package which provides the simple API used here for sampling/inserting values in dense 2D/3D arrays)

I see why only doing it on the three sides might be better but would want to see some sort of validation of that too before changing :-)

@alisterburt
Copy link
Member

also your autoformatter is kinda aggressive and makes PR review a bit tough - are you running black with standard settings or something else?

I'm okay with autoformatting as standard but some of the way this is splitting lines seems a bit much

@jdickerson95
Copy link
Author

I agree the autoformatter was a bit much. I think it was the pre-commit from libtilt so black with standard settings is likely.

I was thinking that the linear interpolation padding is only for the slice extraction so you may still get artefacts if there is a sharp edge present before the ifft. Probably unlikely and would need validation.

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