Skip to content

ENH: add ThreeDPDFCalculator support - #50

Open
joeseaer wants to merge 4 commits into
diffpy:migration-nanofrom
joeseaer:migration-nano-threedpdf
Open

ENH: add ThreeDPDFCalculator support#50
joeseaer wants to merge 4 commits into
diffpy:migration-nanofrom
joeseaer:migration-nano-threedpdf

Conversation

@joeseaer

Copy link
Copy Markdown
Contributor

Hi @sbillinge,

This PR supersedes #49 and ports the 3D PDF calculator work onto the migration-nano branch.

  • Add the CPU ThreeDPDFCalculator implementation.
  • Rename all former PDF3DCalculator identifiers, files, tests, and serialization registrations.
  • Reuse shared PDFCalculator helpers for scattering factors, partial PDF scaling, and RDF normalization.
  • Add 3D grid and radial histogram output, q-window processing, rho0 background scaling, covariance/delta controls, and binary grid export.
  • Add the symmetric 3×3 eigen-solver and ObjCryst REAL=double compatibility fix.
  • Expand the C++ tests to cover the new functionality and migration compatibility.

No legacy PDF3DCalculator names remain in the source code, tests, or filenames.

Testing:

python3 -m SCons test

Running cxxtest tests (218 tests) ... OK!
scons: done building targets.
TEST EXIT CODE: 0

I would really appreciate it if you could spare some time to review this PR.
If you have any feedback or adjustments you'd like to make, feel free to let me know and I'll update it promptly.
Thank you so much for your time and guidance!

@sbillinge sbillinge left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not c++ expert so I have little to say I am afraid, but from a project point of view, this looks great. Much better integrated. Thanks for that @joeseaer.

It would be great if @stevengeorge128 or @SYMSCAE have time to look at it and make any comments about the code implementation, but otherwise I am happy to merge it when we are ready. We still need to do the nanobind ports before we merge this I think.

There is another discussion to be had with @cadenmyers13 and that is whether we make this a separate pack or include it in the cmipdf pack. It reuses much of the PDF code, so putting it into that pack would probably be the best.

@SYMSCAE

SYMSCAE commented Aug 26, 2026

Copy link
Copy Markdown

@sbillinge Code itself looks good to me. I will move diffpy.srreal migration forward so it can sit there better.

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

Successfully merging this pull request may close these issues.

3 participants