Single kernel call for volume and intensity#3963
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the size distribution model-matrix generation so that both per-bin intensity and volume are obtained via a single sasmodels kernel call, removing the prior duplicated/hard-coded volume math path.
Changes:
- Replaces
load_model/DirectModelusage withload_model_info+build_modelandcall_Fqingenerate_model_matrix. - Computes per-bin intensity from returned kernel outputs and stores per-bin volumes on the class for later use.
Comments suppressed due to low confidence (1)
src/sas/sascalc/size_distribution/SizeDistribution.py:193
self._volumesis initialized twice in__init__(once untyped, then again with a type annotation). This duplication is redundant and can confuse readers/type checkers; remove the earlierself._volumes = Noneassignment and keep the single typed initialization.
self.model_matrix: np.ndarray | None = None
self._volumes = None
# Advanced parameters for MaxEnt
self._iterMax: int = 5000
self._skyBackground: float = 1e-6
self._weightType: str = "dI"
self._weightFactor: float = 1.0
self._weightPercent: float = 1.0
self._weights: np.ndarray | None = np.array(data.dy)
self._bin_edges: np.ndarray = np.array([])
self._binDiff: np.ndarray = np.array([])
self._volumes: np.ndarray | None = None
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| call_pars = p.copy() | ||
| _, Fsq, _, volume, volume_ratio = call_Fq(calculator, call_pars) |
jellybean2004
force-pushed
the
SD_remove_hardcode
branch
from
eb993fb to
d921180
Compare
|
|
||
| # Build a single Kernel to compute both intensity and volume per bin | ||
| model_info = load_model_info(self.model) | ||
| model_obj = build_model(model_info) |
There was a problem hiding this comment.
Line 486: model = load_model(self.model) is equivalent to line 499-500. Use model_obj = load_model(self.model) instead.
|
|
||
| # Volume ratio is 1 for solid particles | ||
| if volume_ratio is not None: | ||
| volume *= volume_ratio |
There was a problem hiding this comment.
volume_ratio from call_Fq is never None.
| call_pars = p.copy() | ||
| _, Fsq, _, volume, volume_ratio = call_Fq(calculator, call_pars) | ||
|
|
||
| # Compute intensity using kernel convention: combined_scale = scale / shell_volume |
There was a problem hiding this comment.
This is duplicating code from within sasmodels.kernel.Kernel.Iq. This isn't avoidable in the current implementation, but we should consider this use case when addressing sasmodels #182.
The idea is to return intermediate and derived values from the calculation back to the caller. The current hack to get P@S is that sasmodels.product sets up a kernel.results() function that returns a dictionary with keys P(Q), S(Q), volume, volume_ratio and radius_effective after calling kernel.Iq(). We could extend this to the base Kernel class in sasmodels. Then using call_kernel instead of call_Fq we can get the volume and volume ratio from calculator.results().
Description
Removes hard-coded/duplicated volume math and ensures intensities/volumes come from the model kernel.
Replaces the previous per-bin DirectModel/analytic volume approach with a kernel-based single call.
Builds the sasmodels kernel (from model info) and calls
call_Fqfor each bin to retrieve Fsq and volume. Compute intensity per bin asintensity = (scale/volume) * Fsq + backgroundand store per-bin volumes inself._volumes.Fixes #3418
How Has This Been Tested?
Tested functionality -- same as before.
Review Checklist:
Documentation
Installers
Licensing