Skip to content

q_to_tth & tth_to_q #178

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.

Sign up for GitHub

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

Merged
merged 11 commits into from
Nov 28, 2024
Merged

q_to_tth & tth_to_q #178

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7841ff2
fix docstring and add error messages
yucongalicechen Nov 15, 2024
1b0fa19
add tests
yucongalicechen Nov 15, 2024
4e85b0a
edit error message
yucongalicechen Nov 20, 2024
a33482b
add more tests
yucongalicechen Nov 21, 2024
3efde28
add warning msg for wavelength, edit error msg
yucongalicechen Nov 25, 2024
a53461f
edit warning msg for wavelength, change index error to runtime error
yucongalicechen Nov 25, 2024
2a5a819
add functionality for handling error messages
yucongalicechen Nov 25, 2024
64d8693
remove docstring from private test functions
yucongalicechen Nov 26, 2024
728ff36
add news
yucongalicechen Nov 26, 2024
490486c
add example and utility
yucongalicechen Nov 26, 2024
d481253
edits on docs
yucongalicechen Nov 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 67 additions & 12 deletions tests/diffpy/utils/scattering_objects/test_diffraction_objects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,25 +232,52 @@ def test_diffraction_objects_equality(inputs1, inputs2, expected):


def test_q_to_tth():
# valid q values including edge cases when q=0 or tth=180 after converting
# expected tth values are 2*arcsin(q)
# Valid q values that should result in 0-180 tth values after conversion
# expected tth values are 2*arcsin(q) in degrees
actual = DiffractionObject(wavelength=4 * np.pi)
setattr(actual, "on_q", [[0, 0.2, 0.4, 0.6, 0.8, 1], [1, 2, 3, 4, 5, 6]])
actual_tth = actual.q_to_tth()
expected_tth = [0, 23.07392, 47.15636, 73.73980, 106.26020, 180]
assert np.allclose(actual_tth, expected_tth)


def test_q_to_tth_bad():
# invalid wavelength or q values when arcsin value is not in the range of [-1, 1]
actual = DiffractionObject(wavelength=4 * np.pi)
setattr(actual, "on_q", [[0.6, 0.8, 1, 1.2], [1, 2, 3, 4]])
params_q_to_tth_bad = [
# UC1: user did not specify wavelength
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to not allow missing wavelengths? This makes these DOs harder to use, but also makes them less useful. There may be a middle ground where we allow it, but let the user know that much of the functionality goes away without it. We could also trigger a workflow that requests the wavelength, but still allows users to override that. This would encourage them to enter a wavelength but not insist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes this sounds good. I think user can also directly set attributes so that they can use DO without a wavelength. I would suggest to prompt user inputs in the insert_scattering_quantity function instead of this one when it wants to set arrays on all tth/q/dspace, so that it can avoid calling these functions?
When people use this function directly we would want them to speicify a wavelength, so it's better if we raise an error?

Copy link
Contributor

Choose a reason for hiding this comment

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

For now, how about we just print a warning message if no wavelength is supplied. Something like "INFO: no wavelength has been specified. You can continue to use the DiffractionObject but some of it's powerful features will not be available. To specify a wavelength...."

(
[None, [0, 0.2, 0.4, 0.6, 0.8, 1]],
"Wavelength is not specified. Please provide a valid wavelength, "
"e.g., DiffractionObject(wavelength=0.71).",
),
# UC2: user specified invalid q values that result in tth > 180 degrees
(
[4 * np.pi, [0.2, 0.4, 0.6, 0.8, 1, 1.2]],
"Wavelength * q > 4 * pi. Please check if you entered an incorrect wavelength or q value.",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this behavior is good. The first sentence is a bit mathematical and cryptic. Is there a more "chemist friendly" way of saying it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does "The combination of wavelength and q values is too large" sound good?

Copy link
Contributor

Choose a reason for hiding this comment

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

How about: "the supplied q-array and wavelength will result in an impossible two-theta. Please check these values and re-instantiate the DiffractionObject"

),
# UC3: user specified a wrong wavelength that result in tth > 180 degrees
(
[100, [0, 0.2, 0.4, 0.6, 0.8, 1]],
"Wavelength * q > 4 * pi. Please check if you entered an incorrect wavelength or q value.",
),
# UC4: user specified an empty q array
Copy link
Contributor

Choose a reason for hiding this comment

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

It is ok to specify no q-array (you can instantiate on tth for example). Presumably this is only an error if the the DO is being created with q data. So the behavior is ok, but we may want to tweak the error message. I think a more general error is if the x and y arrays are not the same length, whether they are q, tth or d. I suggest to handle that more general case?

Finally, do we also want to specify what error is raised as well as the message?

([4 * np.pi, []], "Q array is empty. Please provide valid q values."),
# UC5: user specified a non-numeric value in q array
(
[4 * np.pi, [0, 0.2, 0.4, 0.6, 0.8, "invalid"]],
"Invalid value found in q array. Please ensure all values are numeric.",
),
]


@pytest.mark.parametrize("inputs, expected", params_q_to_tth_bad)
def test_q_to_tth_bad(inputs, expected):
actual = DiffractionObject(wavelength=inputs[0])
setattr(actual, "on_q", [inputs[1], [1, 2, 3, 4, 5, 6]])
with pytest.raises(ValueError):
actual.q_to_tth()


def test_tth_to_q():
# valid tth values including edge cases when tth=0 or 180 degree
# Valid tth values between 0-180 degrees
# expected q vales are sin15, sin30, sin45, sin60, sin90
actual = DiffractionObject(wavelength=4 * np.pi)
setattr(actual, "on_tth", [[0, 30, 60, 90, 120, 180], [1, 2, 3, 4, 5, 6]])
Expand All @@ -259,11 +286,39 @@ def test_tth_to_q():
assert np.allclose(actual_q, expected_q)


def test_tth_to_q_bad():
# invalid tth value of > 180 degree
actual = DiffractionObject(wavelength=4 * np.pi)
setattr(actual, "on_tth", [[0, 30, 60, 90, 120, 181], [1, 2, 3, 4, 5, 6]])
with pytest.raises(ValueError):
params_tth_to_q_bad = [
# UC1: user did not specify wavelength
(
[None, [0, 30, 60, 90, 120, 180]],
"Wavelength is not specified. Please provide a valid wavelength, "
"e.g., DiffractionObject(wavelength=0.71).",
),
# UC2: user specified an invalid tth value of > 180 degrees
(
[4 * np.pi, [0, 30, 60, 90, 120, 181]],
"Two theta exceeds 180 degrees. Please check the input values for errors.",
),
# UC3: user did not specify wavelength and specified invalid tth values
(
[None, [0, 30, 60, 90, 120, 181]],
"Wavelength is not specified. Please provide a valid wavelength, "
"e.g., DiffractionObject(wavelength=0.71).",
),
# UC4: user specified an empty two theta array
([4 * np.pi, []], "Two theta array is empty. Please provide valid two theta values."),
# UC5: user specified a non-numeric value in two theta array
(
[4 * np.pi, [0, 30, 60, 90, 120, "invalid"]],
"Invalid value found in two theta array. Please ensure all values are numeric.",
),
]


@pytest.mark.parametrize("inputs, expected", params_tth_to_q_bad)
def test_tth_to_q_bad(inputs, expected):
actual = DiffractionObject(wavelength=inputs[0])
setattr(actual, "on_tth", [inputs[1], [1, 2, 3, 4, 5, 6]])
with pytest.raises(ValueError, match=expected):
actual.tth_to_q()


Expand Down
Loading