fix: update functions related to `diffpy.utils` update #151

yucongalicechen · 2025-01-03T00:31:39Z

closes #129

…operly

yucongalicechen · 2025-01-03T00:37:36Z

src/diffpy/labpdfproc/functions.py


 RADIUS_MM = 1
 N_POINTS_ON_DIAMETER = 300
-TTH_GRID = np.arange(1, 180.1, 0.1)
+TTH_GRID = np.round(np.arange(1, 180.1, 0.1), decimals=1)


np.arrange causes the value at 180 degree to go above 180 slightly because of floating-point precision errors. I think we can round every value to 1 decimal place here.

I ran the interpolation and coefficient lists using np.arrange though... but after rerunning a few angles I believe this rounding error is so small that it can be ignored

mmm., this seems like a slightly dramatic fix to a small edge-case. Is there not a less intrusive fix for this? Let's think about this a bit more.

Yeah, maybe I can round down 180 only?

that's what I had in mind.

src/diffpy/labpdfproc/tools.py

yucongalicechen · 2025-01-03T00:41:52Z

@sbillinge ready for review. This PR includes all updates from diffpy.utils. No significant functionality changes. But it's causing a conflict because I removed the mud_calculator.

sbillinge

exciting! please see inline.

sbillinge · 2025-01-03T14:06:28Z

src/diffpy/labpdfproc/functions.py


 RADIUS_MM = 1
 N_POINTS_ON_DIAMETER = 300
-TTH_GRID = np.arange(1, 180.1, 0.1)
+TTH_GRID = np.round(np.arange(1, 180.1, 0.1), decimals=1)


mmm., this seems like a slightly dramatic fix to a small edge-case. Is there not a less intrusive fix for this? Let's think about this a bit more.

src/diffpy/labpdfproc/functions.py

src/diffpy/labpdfproc/tools.py

tests/test_functions.py

sbillinge · 2025-01-03T20:22:24Z

you will have to merge in main to resolve the conflict (or add the mud calculator back....)

yucongalicechen · 2025-01-14T18:33:16Z

src/diffpy/labpdfproc/functions.py


 RADIUS_MM = 1
 N_POINTS_ON_DIAMETER = 300
 TTH_GRID = np.arange(1, 180.1, 0.1)
+# Round down the last element if it's slightly above 180 due to floating point precision
+TTH_GRID[-1] = 180


round down 180 only

maybe 180.00 so it is clear you want to keep it as a float?

codecov · 2025-01-14T18:35:02Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.27%. Comparing base (a9fbbe5) to head (402677b).
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
- Coverage   99.31%   99.27%   -0.05%     
==========================================
  Files           6        5       -1     
  Lines         293      275      -18     
==========================================
- Hits          291      273      -18     
  Misses          2        2

Files with missing lines	Coverage Δ
tests/conftest.py	`100.00% <100.00%> (ø)`
tests/test_fixtures.py	`100.00% <100.00%> (ø)`
tests/test_functions.py	`100.00% <100.00%> (ø)`
tests/test_tools.py	`98.61% <100.00%> (ø)`

yucongalicechen · 2025-01-14T18:37:10Z

@sbillinge ready for another review. I fixed the TTH_GRID and resolved the conflicts. I will open another PR on fixing docstrings and tests for username orcid etc. once this is merged.

sbillinge

I didn't quite finish the review and have to run, but I am posting this now so you can work on it.

sbillinge · 2025-01-14T19:50:15Z

news/utils-updates.rst

+
+**Changed:**
+
+* Updated functions related to changes in `diffpy.utils` version 3.6.0.


Can you try and think more precisely about how to say this. "Updated" is the same as "changed" so the message contains close to zero information.....

sbillinge · 2025-01-14T19:51:15Z

src/diffpy/labpdfproc/functions.py


 RADIUS_MM = 1
 N_POINTS_ON_DIAMETER = 300
 TTH_GRID = np.arange(1, 180.1, 0.1)
+# Round down the last element if it's slightly above 180 due to floating point precision
+TTH_GRID[-1] = 180


maybe 180.00 so it is clear you want to keep it as a float?

sbillinge · 2025-01-14T19:52:18Z

src/diffpy/labpdfproc/tools.py

-from diffpy.utils.scattering_objects.diffraction_objects import QQUANTITIES, XQUANTITIES
-from diffpy.utils.tools import get_package_info, get_user_info
+from diffpy.utils.diffraction_objects import ANGLEQUANTITIES, QQUANTITIES, XQUANTITIES
+from diffpy.utils.tools import check_and_build_global_config, compute_mud, get_package_info, get_user_info

 WAVELENGTHS = {"Mo": 0.71, "Ag": 0.59, "Cu": 1.54}


I think we want a few more sig figs on these

I found for Mo and Cu the values should be around 0.7107 and 1.5406. I couldn't find the values for Ag though..

sbillinge · 2025-01-14T19:56:19Z

src/diffpy/labpdfproc/tools.py

-    config = get_user_info(config)
-    args.username = config["username"]
-    args.email = config["email"]
+    check_and_build_global_config()


since here we are allowing people to pass in this info as an arg, probably the workflow should be

if the arg is set, use the arg and don't even run the config updater

if the arg is not set, run the config updater

run get_user_info passing in the args if they are not None.

sbillinge · 2025-01-14T19:56:56Z

src/diffpy/labpdfproc/tools.py

-    args.email = config["email"]
+    check_and_build_global_config()
+    config = get_user_info(owner_name=args.username, owner_email=args.email)
+    args.username = config["owner_name"]


is there a reason you are not using config.get() here? That is usually better practice.

yucongalicechen · 2025-01-14T21:18:54Z

news/utils-updates.rst

+
+**Changed:**
+
+* Functions related to diffraction_objects and tools modules in `diffpy.utils` to reflect changes in version 3.6.0.


better wording..?

not really. None of the users reading this will have any idea about diffpy.utils 3.6.0.

Something like

* Functions that use DiffractionObject` in `diffpy.utils` to follow the new API.

src/diffpy/labpdfproc/functions.py

yucongalicechen · 2025-01-14T21:21:10Z

src/diffpy/labpdfproc/tools.py


-WAVELENGTHS = {"Mo": 0.71, "Ag": 0.59, "Cu": 1.54}
+WAVELENGTHS = {"Mo": 0.71073, "Ag": 0.55941, "Cu": 1.5406}


Need a little help on the value for Ag. I found it here https://x-server.gmca.aps.anl.gov/cgi/www_dbli.exe?x0hdb=waves but it's a bit off from our original value..

"Cu", "CuKa1", "CuKa1Ka2"
CuKa1 : 1.54056
CuKa1Ka2 : =(1.54056*2 + 1.544398)/3 = 1.54184
Cu : 1.542

@yucongalicechen will need docs to explain what is going on.

src/diffpy/labpdfproc/tools.py

yucongalicechen · 2025-01-14T21:23:44Z

@sbillinge ready for some more feedback. The error in the tests is related to the wavelength for Ag.

sbillinge

please see inline

sbillinge · 2025-01-14T22:28:07Z

news/utils-updates.rst

+
+**Changed:**
+
+* Functions related to diffraction_objects and tools modules in `diffpy.utils` to reflect changes in version 3.6.0.


not really. None of the users reading this will have any idea about diffpy.utils 3.6.0.

Something like

* Functions that use DiffractionObject` in `diffpy.utils` to follow the new API.

sbillinge · 2025-01-14T22:39:07Z

src/diffpy/labpdfproc/tools.py


-WAVELENGTHS = {"Mo": 0.71, "Ag": 0.59, "Cu": 1.54}
+WAVELENGTHS = {"Mo": 0.71073, "Ag": 0.55941, "Cu": 1.5406}


"Cu", "CuKa1", "CuKa1Ka2"
CuKa1 : 1.54056
CuKa1Ka2 : =(1.54056*2 + 1.544398)/3 = 1.54184
Cu : 1.542

sbillinge · 2025-01-14T22:39:24Z

src/diffpy/labpdfproc/tools.py


-WAVELENGTHS = {"Mo": 0.71, "Ag": 0.59, "Cu": 1.54}
+WAVELENGTHS = {"Mo": 0.71073, "Ag": 0.55941, "Cu": 1.5406}


@yucongalicechen will need docs to explain what is going on.

yucongalicechen added 6 commits January 2, 2025 19:05

docs: add news

da3340b

fix: correct imports from utils and initialize diffraction objects pr…

8264f17

…operly

fix: update keys in home config file to match changes in utils

f9a9eb1

fix: update diffraction objects in functions

eb5b771

fix: updates for fixtures and tools

e79a2fd

fix: define TTH_GRID more elegantly

84d0e33

yucongalicechen commented Jan 3, 2025

View reviewed changes

src/diffpy/labpdfproc/tools.py Show resolved Hide resolved

yucongalicechen commented Jan 3, 2025

View reviewed changes

src/diffpy/labpdfproc/tools.py Outdated Show resolved Hide resolved

sbillinge reviewed Jan 3, 2025

View reviewed changes

yucongalicechen added 2 commits January 14, 2025 13:28

fix: tweak TTH_GRID and add back check_and_build_global_config

0cad788

merge from main

0534810

yucongalicechen commented Jan 14, 2025

View reviewed changes

sbillinge reviewed Jan 14, 2025

View reviewed changes

yucongalicechen added 2 commits January 14, 2025 15:47

doc: clarify news, clarify TTH_GRID

b479cf2

fix: updated wavelengths, workflow for loading user info

5fecd14

yucongalicechen commented Jan 14, 2025

View reviewed changes

src/diffpy/labpdfproc/functions.py Show resolved Hide resolved

yucongalicechen commented Jan 14, 2025

View reviewed changes

src/diffpy/labpdfproc/tools.py Show resolved Hide resolved

yucongalicechen closed this Jan 14, 2025

yucongalicechen reopened this Jan 14, 2025

sbillinge reviewed Jan 14, 2025

View reviewed changes

docs: update news and wavelength

402677b

yucongalicechen mentioned this pull request Jan 14, 2025

fix: update wavelengths #153

Closed

sbillinge merged commit 08032ab into diffpy:main Jan 14, 2025
4 of 5 checks passed

yucongalicechen deleted the utils-update branch January 15, 2025 17:59

yucongalicechen mentioned this pull request Jan 15, 2025

getting started documentation #114

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update functions related to `diffpy.utils` update #151

fix: update functions related to `diffpy.utils` update #151

yucongalicechen commented Jan 3, 2025

yucongalicechen Jan 3, 2025

yucongalicechen Jan 3, 2025

sbillinge Jan 3, 2025

yucongalicechen Jan 3, 2025

sbillinge Jan 5, 2025

yucongalicechen commented Jan 3, 2025

sbillinge left a comment

sbillinge Jan 3, 2025

sbillinge commented Jan 3, 2025

yucongalicechen Jan 14, 2025

sbillinge Jan 14, 2025

codecov bot commented Jan 14, 2025 •

edited

Loading

yucongalicechen commented Jan 14, 2025 •

edited

Loading

sbillinge left a comment

sbillinge Jan 14, 2025

sbillinge Jan 14, 2025

sbillinge Jan 14, 2025

yucongalicechen Jan 14, 2025

sbillinge Jan 14, 2025

sbillinge Jan 14, 2025

yucongalicechen Jan 14, 2025

sbillinge Jan 14, 2025

yucongalicechen Jan 14, 2025

sbillinge Jan 14, 2025

sbillinge Jan 14, 2025

yucongalicechen commented Jan 14, 2025

sbillinge left a comment

sbillinge Jan 14, 2025

sbillinge Jan 14, 2025

sbillinge Jan 14, 2025


		Changed:

		* Updated functions related to changes in `diffpy.utils` version 3.6.0.


		Changed:

		* Functions related to diffraction_objects and tools modules in `diffpy.utils` to reflect changes in version 3.6.0.


		WAVELENGTHS = {"Mo": 0.71, "Ag": 0.59, "Cu": 1.54}
		WAVELENGTHS = {"Mo": 0.71073, "Ag": 0.55941, "Cu": 1.5406}

fix: update functions related to diffpy.utils update #151

fix: update functions related to diffpy.utils update #151

Conversation

yucongalicechen commented Jan 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yucongalicechen commented Jan 3, 2025

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sbillinge commented Jan 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 14, 2025 • edited Loading

Codecov Report

yucongalicechen commented Jan 14, 2025 • edited Loading

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yucongalicechen commented Jan 14, 2025

sbillinge left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fix: update functions related to `diffpy.utils` update #151

fix: update functions related to `diffpy.utils` update #151

codecov bot commented Jan 14, 2025 •

edited

Loading

yucongalicechen commented Jan 14, 2025 •

edited

Loading