Number of molecules input; S_eff(q) output#15
Merged
Conversation
added 2 commits
June 21, 2026 18:13
For a periodic box of N identical molecules (the `direct` scheme, optionally XTC-averaged), `--num-molecules N` outputs the apparent structure factor and the at-concentration single-molecule form factor: S_eff(q) = |Σ_m F_m(q)|² / Σ_m |F_m(q)|² (exact, no approximation) P(q) = (1/N) · Σ_m |F_m(q)|² The box is split into N contiguous equal-size molecules and each molecule's amplitude F_m is accumulated alongside the self term Σ_m|F_m|² in DirectTransform::vacuum_intensities. Both use the box-commensurate q-vectors, so they are PBC-invariant (no unwrapping). S_eff and P are derived from the self term in Pripps::intensity and emitted as the CSV columns `p,s_eff` (and the Python .p/.s_eff getters). P(q) is the at-concentration form factor — for a flexible molecule it shifts with crowding, unlike the dilute P an experiment divides by. Guards (fail up front, before any trajectory pass): direct scheme only, vacuum only, box atom count a multiple of N, and every M-atom block must share the first block's atom-kind sequence (rejects a count that splits molecules or doesn't match the box composition). --num-molecules conflicts with --fit and runs on the CPU even with the gpu feature (the GPU kernel emits only the total |Σf|²); both are noted. XTC averaging assumes a fixed box.
Extend the WGSL direct kernel to emit the per-molecule self term Σ_m|F_m|² alongside the box intensity (one workgroup per q-vector, threads stride over whole molecules), so the effective structure factor runs on the GPU instead of falling back to the CPU. Output is two f32 per q-vector; the whole-box path (no --num-molecules) is unchanged. Route DirectGpu + --num-molecules to the GPU, with the existing CPU fallback when no adapter is available. Cleanups: route average_duplicates and the 5-column averager through the shared average_by_qnorm (removing the triplicated q-binning/sort), and add an OUT_BYTES_PER_Q constant for the GPU output stride. Rename the internal Intensity carrier monomer_self -> self_term to match the local accumulators and drop the stale "monomer" naming. Tests: GPU-vs-CPU parity for the self term, and a trajectory test pinning S_eff as the ratio of frame-averaged box intensity to frame-averaged self term (ratio of averages, not the average of per-frame S_eff).
There was a problem hiding this comment.
Pull request overview
This PR adds an effective structure factor workflow for periodic boxes containing multiple identical molecules, exposing it via a new --num-molecules / num_molecules argument and extending both CPU and GPU direct-transform paths to produce the additional outputs.
Changes:
- Add
--num-molecules/num_moleculesto compute per-molecule self term and deriveP(q)andS_eff(q)(CPU + GPU). - Refactor duplicate-|q| averaging into a generic
average_by_qnormutility used across 2-col and multi-col outputs. - Extend CLI CSV and Python bindings to expose the new
p/s_effoutputs when available, and document usage in the README.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib.rs |
Adds Intensity fields for P(q) / S_eff(q), validates --num-molecules, derives outputs from self_term, adds tests. |
src/explicit.rs |
Computes both box intensity and per-molecule self term on CPU; introduces average_by_qnorm. |
src/gpu.rs |
Extends WGSL kernel + host plumbing to emit both I_box and self term; averages both by ` |
src/trajectory.rs |
Updates trajectory averaging to also accumulate/normalize the self term for ratio-of-averages S_eff. |
src/cli.rs |
Adds CLI flag and conditionally appends p,s_eff columns to CSV output. |
pripps-py/src/lib.rs |
Adds Python p and s_eff attributes and num_molecules argument plumbing. |
README.md |
Documents theory, constraints, and CLI/Python usage examples for --num-molecules. |
src/multipole/mod.rs |
Uses ..Default::default() to accommodate new Intensity fields. |
src/debye.rs |
Uses ..Default::default() to accommodate new Intensity fields. |
benches/intensity.rs |
Updates benchmark calls for the extended Pripps::intensity signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+374
to
+379
| if !structure.pos.len().is_multiple_of(n) { | ||
| return Err(Error::validation(format!( | ||
| "box atom count ({}) is not a multiple of --num-molecules ({n})", | ||
| structure.pos.len() | ||
| ))); | ||
| } |
Comment on lines
+231
to
239
| // Normalize the accumulated sums by the total weight to get the weighted | ||
| // averages (`total` and the self term divided by the same weight). | ||
| for acc in mean | ||
| .total | ||
| .iter_mut() | ||
| .chain(mean.self_term.iter_mut().flatten()) | ||
| { | ||
| *acc /= weight_sum; | ||
| } |
added 3 commits
June 22, 2026 10:08
trajectory_average divides accumulated curves by the consumed weight sum. read_weights validates the full set, but striding can select a zero-sum subset of signed weights, which would turn every average into NaN; reject it explicitly instead. The companion review note about an empty box (0 atoms) yielding NaN S_eff is already covered upstream — Structure::from_file rejects atomless inputs before the --num-molecules guard runs — so no extra check is needed there; add a regression test and a clarifying comment.
Replace the terse --num-molecules block with a proper derivation (GitHub LaTeX): per-molecule amplitude, the self/cross split of the box intensity, the exact P(q) and S_eff(q) definitions, and the decoupling relation S_eff = 1 + beta(S-1) with the beta-node inversion caveat. Reference the primary source for the decoupling approximation (Kotlarchyk & Chen, J. Chem. Phys. 79, 2461 (1983)).
GitHub's markdown pairs the underscores in adjacent inline $...$ spans as emphasis, stripping them and breaking the math (e.g. R_m, S_eff). Switch inline math to the backtick-protected $`...`$ form, whose contents are immune to markdown; display blocks are unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new feature for calculating and outputting the effective structure factor (
S_eff(q)) and the at-concentration single-molecule form factor (P(q)) for periodic boxes containing multiple identical molecules. This is enabled via a new--num-moleculesCLI/Python argument, and is supported in both CPU and GPU code paths. The PR also refactors internal averaging utilities and updates the output format to include the new data when requested.The most important changes are:
New Structure Factor Feature:
S_eff(q)andP(q)by specifying the number of molecules in the box (--num-moleculesfor CLI,num_moleculesfor Python). This splits the box into contiguous, equal-sized molecules and accumulates both the box amplitude and the per-molecule self term, enabling calculation of inter-particle structure factors. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16] [17]Output and API Enhancements:
pands_effcolumns/attributes when the structure factor calculation is performed, ensuring the output format adapts dynamically to the selected options. [1] [2] [3] [4]Internal Refactoring and Utilities:
average_by_qnormutility, which is now used for both 2-column and multi-column data, improving maintainability and consistency. [1] [2]GPU Kernel and Data Path Updates:
Documentation:
README.mdwith a clear explanation and usage examples for the new structure factor feature, including CLI and Python usage, and the scientific background for interpreting the new outputs.These changes collectively enable new scientific analyses for crowded molecular systems and improve the usability and extensibility of the codebase.