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

Scattering angle for reflectometry #528

Merged
merged 8 commits into from
Jul 8, 2024
Merged

Scattering angle for reflectometry #528

merged 8 commits into from
Jul 8, 2024

Conversation

jl-wynen
Copy link
Member

First step for scipp/essreflectometry#54

This computes $\gamma$ as defined in https://doi.org/10.1016/j.nima.2016.03.007. ESSreflectometry will subtract $\omega$ to obtain $\theta$ because, this way, scattering_angle_in_yz_plane is more generic.

@nvaytet nvaytet self-assigned this Jun 26, 2024


@pytest.mark.parametrize('polar', [np.pi / 3, np.pi / 2, 2 * np.pi / 3, np.pi])
def test_scattering_angle_in_yz_plane_reproduces_angles_azimuth_greater_pi(
Copy link
Member

Choose a reason for hiding this comment

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

I may be wrong but it looks to me like this test is doing exactly the same as the previous test? Is it that the values in the parametrize should be different?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right. This was copied from a test for the regular scattering angles that includes the azimuthal angle. But I removed that angle...

y += sc.dot(scattered_beam, ey).to(dtype=elem_dtype(wavelength), copy=False)
y = sc.abs(y, out=y)
z = sc.dot(scattered_beam, ez).to(dtype=elem_dtype(y), copy=False)
return sc.atan2(y=y, x=z, out=y)
Copy link
Member

Choose a reason for hiding this comment

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

Looks good but same question as before: have you checked that it gives the same results as before?

I guess we also expect a difference because we used asin in the reflectometry code, but if we replace that by an atan we should get identical results?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not give the same results because of the sign error in the old code. I did not see any difference from the asin / atan change. But I am still investigating.

@jl-wynen
Copy link
Member Author

See scipp/essreflectometry#55

@jl-wynen jl-wynen merged commit 525c424 into main Jul 8, 2024
4 checks passed
@jl-wynen jl-wynen deleted the theta-reflectometry branch July 8, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants