@sbillinge Please see if this is what we want.

@SYMSCAE This PR is still failing. I suspect it is because we haven't updated the dependencies for this package (libdiffpy and pyobjcryst). So, in order to make it build properly, we might need to update its dependency so that we could test whether it would work. @sbillinge We might also open migration branch on libdiffpy and pyobjcryst so that @SYMSCAE and @joeseaer would work on that.

@SYMSCAE This PR is still failing. I suspect it is because we haven't updated the dependencies for this package (libdiffpy and pyobjcryst). So, in order to make it build properly, we might need to update its dependency so that we could test whether it would work. @sbillinge We might also open migration branch on libdiffpy and pyobjcryst so that @SYMSCAE and @joeseaer would work on that.

Migration work in those repos is on migration-nano branches. I am not sure about pyobjcryst but we can check. Please coordinate with earlier work fine by @SYMSCAE and others (I don't remember exactly)

@SYMSCAE @stevenhua0320 we need to merge this to move the 3D PDF calculator forward. Please can we work on getting the dependency issue resolved so we can move forward? This is a bit urgent.

@SYMSCAE Let's first work on pyobjcryst as I believe there is a migration-nano for migration purpose. For this week you could work on that package and bug me if you are ready for the review.

@SYMSCAE Let's first work on pyobjcryst as I believe there is a migration-nano for migration purpose. For this week you could work on that package and bug me if you are ready for the review.

Hi Steven, I believe we don’t need to touch pyobjcryst. The only change in upstream library is libdiffpy, which is requiring a new conda release. After that we should be good to pass rest of the tests.

Hi @SYMSCAE @stevenhua0320 @sbillinge,

I have opened diffpy/libdiffpy#50, which ports ThreeDPDFCalculator onto the migration-nano branch. The full C++ test suite passes locally (218 tests).

Once #50 is merged and #89 provides a stable nanobind base, I’ll add the corresponding ThreeDPDFCalculator bindings in a follow-up PR based on the updated migration-nano branch.

Please feel free to let me know if I can help with any related issues or reviews in the meantime. I’d be happy to assist.

@joeseaer it could help us if you could lead the nanobind migration as we are still 1 month away from being back in session and this is not the focus of @SYMSCAE and @stevengeorge128, our c++ folks, at the moment.

Basically, we want to figure out how to do the testing on libdiffpy migration-nano branch to make sure we didn't break anything (and did it in the optimal best way), then merge that into main and do a release. Then we need to do the same for diffpy.srreal. Finally we will need to take what we learned to pdffit2.

@joeseaer would you be able to take that on and report back all the steps and the results?

Codecov Report

❌ Patch coverage is 98.84868% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.67%. Comparing base (495c27e) to head (ade2fce).

@@                Coverage Diff                 @@
##           migration-nano      #89      +/-   ##
==================================================
+ Coverage           98.52%   98.67%   +0.15%     
==================================================
  Files                  20       27       +7     
  Lines                3248     3844     +596     
==================================================
+ Hits                 3200     3793     +593     
- Misses                 48       51       +3     

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sbillinge I’ve aligned some approaches with diffpy/pyobjcryst#101 such as moving build system to cmake and made some workaround to pin ci to run on specific commits so we can work more flexibly in the future. Please see if this is what we want.

@sbillinge pr ready for review. We can review behavioral changes and move on to make better use of nanobind features from here. Calculations from my end are consistent with the previous Boost.Python version.

Btw @SYMSCAE we normally like to have screenshots of your comparison tests so we have s better idea of the extent of what you did and what you got. A bit more than "my valuations were consistent"

I will merge this. There is a lot of hard coded things that will go stale with time, such as python versions specified in CMAKE recipes and so on. This will make it difficult to maintain moving forward. Is it possible to rationalize things as a future issue/project so that there is a well defined set of steps to do updates each year? Action item: make an issue to work on rationalizing this, something like that?

Sure it is possible. I will switch to git submodules for commit pins (this requires a one-line change in release scripts, so maybe we should leave it for future commits?) and centralize build system version configs. For verifications, here's some visualizations.

Previous discrepancies are proven to be build flag issues (fast math etc.), the results below show negligible differences when using the same build flag for libdiffpy.

Interesting results, thanks @SYMSCAE .

1. please make an issue and let's do the centralization of the build stuff on a separate PR.
2. It is interesting that there were small changes that depended on the flags. When you changed the flags we recovered the old Boost results, so that gives a lot of confidnec that the nanobind workk is good. Let's make a another issue to explore the best use of flags to give good accuracy in general.