Skip to content

ENH:Add 3D PDF calculator support#49

Open
joeseaer wants to merge 6 commits into
diffpy:3DPDFfrom
joeseaer:main
Open

ENH:Add 3D PDF calculator support#49
joeseaer wants to merge 6 commits into
diffpy:3DPDFfrom
joeseaer:main

Conversation

@joeseaer

@joeseaer joeseaer commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Hi @sbillinge @dragonyanglong ,

This PR adds a CPU PDF3DCalculator implementation to libdiffpy. The calculator
produces 3D real-space PDF data on a cubic grid, exposes radial histogram data,
supports binary dense-grid export, q-window post-processing, rho0 background
scaling, and 3D covariance delta controls.

It also includes the small supporting changes needed by the calculator:
a symmetric 3x3 eigen-solver helper in R3linalg, a PDFCalculator helper for
partial PDF scale, and an ObjCryst translation-vector type compatibility fix
for builds where ObjCryst::REAL is double.

I tested the libdiffpy build locally with ObjCryst/GSL/Boost available.

@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 didn't see any tests. Are there tests?

Can we also try and think about if it makes sense to make shared functions for anything to avoid copy-paying code from the PDF calculator? The more we can separate into shared functions the less duplication we have to maintain in the future

Comment thread src/diffpy/srreal/PDF3DCalculator.hpp Outdated
*
* libdiffpy by DANSE Diffraction group
* Simon J. L. Billinge
* (c) 2009 The Trustees of Columbia University

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.

Please fix date and address. I am not sure we actually need this text block any more as we do the copyright and attribution differently do maybe just delete the whole block?

@joeseaer

joeseaer commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks Simon. I updated the PR to address these review comments.

I removed the copied legacy copyright/header blocks from the new
PDF3DCalculator.cpp and PDF3DCalculator.hpp files.

I also refactored the shared structure-factor and normalization logic so
PDF3DCalculator now reuses PDFCalculator helpers for site scattering factors,
partial PDF scale, and RDF scale instead of maintaining its own duplicate cache.
The duplicated 3D post-processing path used by get3DPDF and exportGrid3DBinary
has also been consolidated.

The 3D grid deposition, spherical q-window, and delta-shell preprocessing remain
inside PDF3DCalculator because they operate on the 3D grid representation and do
not map directly to the existing 1D PDFCalculator routines.

I verified the updated libdiffpy build locally with scons -j2.

@sbillinge

Copy link
Copy Markdown
Contributor

This is great. Thanks. Please can you paste the results of running tests locally?

Also, possibly on a new branch, please could you try and merge migration-nano into this branch and see if the tests still pass.. Actually, I would rather merge your changes into that branch so that it is coded at the version of the next release

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