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

Possible error in _alg2d_fmp.py #19

Closed
brijn02 opened this issue May 10, 2023 · 2 comments
Closed

Possible error in _alg2d_fmp.py #19

brijn02 opened this issue May 10, 2023 · 2 comments

Comments

@brijn02
Copy link
Contributor

brijn02 commented May 10, 2023

Dear @paulmueller,

I am currently working on my Bachelor Thesis and I used your code as a basis for different testings. I discovered some inconsistencies in the _alg2d_fmp.py file. The function fourier_map_2d adds a filter to Fcomp to only include the wave with a wavenumber smaller then \sqrt{2} k_m in line 255. However, I believe this conditions isn't applied correctly. The results are now inconsistent for different units (one can apply dimensionless code by compensating for the resolution on the detector or compute the results with units). I believe the mistake is in line 255 and it should be corrected as follows:

Fcomp[np.where(np.sqrt(kinx * * 2 + kiny * * 2) > np.sqrt(2) * km)] = 0

With the square root added the units are correct in that equation. Do you agree?

Thanks in advance!

Best regards,

Bernd

@paulmueller
Copy link
Member

What you are writing makes a lot of sense to me!

In the current implementation, it seems like the Fourier space is unnecessarily cropped.

A slightly faster approach would probably be to square km like so:

Fcomp[np.where((kinx**2 + kiny**2) > 2 * km**2)] = 0

Would you like to create a pull request and officially contribute to ODTbrain?

@brijn02
Copy link
Contributor Author

brijn02 commented May 10, 2023

Dear @paulmueller,

Thank you for your fast response. A contribution would be nice! I will create a pull request tomorrow.

Thanks again.

Best regards,

Bernd

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

No branches or pull requests

2 participants