-
Notifications
You must be signed in to change notification settings - Fork 30
Adding torus elliptical shell model #704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
IndigoCarmine
wants to merge
20
commits into
SasView:master
Choose a base branch
from
IndigoCarmine:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 11 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
54f8635
Add torus elliptical shell model implementation in C and Python
IndigoCarmine 9cf010f
Update author information in torus elliptical shell model
IndigoCarmine 1c758a0
Add schematic image for torus elliptical shell geometry and update do…
IndigoCarmine c5e33d4
fix the torus elliptical shell model by removing the unnecessary fmax…
IndigoCarmine 0392af7
Fix calculation in F_torus function by correcting int_r_delta assignment
IndigoCarmine 376dd24
Remove mistake condition in F_torus function for Q_sin_theta check
IndigoCarmine 814325d
Initialize variables in F_torus and Iq functions.
IndigoCarmine 2cb3bdd
fix volume calculation in form_volume and update Iq function to corre…
IndigoCarmine 3b2dd1c
correct file name for J0 function in source list
IndigoCarmine 798a466
add test cases for torus elliptical shell model
IndigoCarmine e93a806
remove outdated reference to M. J. Hollamby in documentation
IndigoCarmine e03af83
Fix: Fq can calculate both F1 and F2; remove division by form_volume
IndigoCarmine 3149883
Add: volume and radius calculations in torus elliptical shell model
IndigoCarmine 490b663
Updated test values to reflect changes correcting scale and FormVolum…
IndigoCarmine c0eac41
feat: separate nu value into core_nu and shell_nu.
IndigoCarmine 9d22ce9
Update torus elliptical shell geometry image
IndigoCarmine c9072a9
refactor: update form_volume calculation to use core and shell nu val…
IndigoCarmine 2968487
feat: add case for major radius in radius_effective function and upda…
IndigoCarmine bc8c875
feat: add max_radius function and update radius_effective_models list
IndigoCarmine 85e1955
refactor: rename core_radius to radius_core and update related parame…
IndigoCarmine File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| static double form_volume(double radius, double core_radius, double thickness, | ||
| double nu) { | ||
| double ao = core_radius + thickness; | ||
| double bo = nu * ao; | ||
| double area = M_PI * ao * bo; | ||
| return 2.0 * M_PI * radius * area; | ||
| } | ||
|
|
||
| static double F_torus(double Q, double theta, double R, double x, double nu, | ||
| double delta_eta) { | ||
| // integral_{R - x}^{R + x} | ||
| // 4 * pi * r * J0(Q * r * sin(theta)) * ganmma | ||
| // sinx_x(Q * ganmma * cos(theta)) dr | ||
|
|
||
| // Set lower integration bound to R-x (not min(R-x,0)), since a torus has a | ||
| // hole and R-x > 0 is always valid. | ||
|
|
||
| double gamma = 0, int_r_delta = 0, r = 0, f_total = 0; | ||
| const double square_x = square(x); | ||
| const double Q_sin_theta = Q * sin(theta); | ||
| const double Q_cos_theta = Q * cos(theta); | ||
|
|
||
| f_total = 0.0; | ||
|
|
||
| for (int i = 0; i < GAUSS_N; i++) { | ||
| // translate a point in[-1, 1] to a point in[R - x, R + x] | ||
| r = GAUSS_Z[i] * x + R; | ||
|
|
||
| gamma = nu * sqrt(square_x - square(r - R)); | ||
|
|
||
| int_r_delta = | ||
| r * sas_J0(r * Q_sin_theta) * sas_sinx_x(Q_cos_theta * gamma) * gamma; | ||
| f_total += GAUSS_W[i] * int_r_delta; | ||
| } | ||
|
|
||
| return 4.0 * M_PI * f_total * x * | ||
| delta_eta; // multiply by x to get the integral over [R - x, R + x] | ||
| } | ||
|
|
||
| static double Iq(double q, double radius, double core_radius, double thickness, | ||
| double nu, double sld_core, double sld_shell, | ||
| double sld_solvent) { | ||
| // integral_{0}^{pi / 2} | ||
| // | F_torus(Q, theta, R, core_radius + thickness, nu, sld_shell - | ||
| // sld_solvent) // shell | ||
| // - F_torus(Q, theta, R, core_radius, nu, sld_core - sld_solvent) // | ||
| // core | ||
| // |^2 dtheta | ||
| double F_diff = 0.0, theta = 0.0, I_total = 0.0; | ||
|
|
||
| for (int i = 0; i < GAUSS_N; i++) { | ||
| // translate a point in[-1, 1] to a point in[0, pi / 2] | ||
| theta = GAUSS_Z[i] * M_PI_4 + M_PI_4; | ||
|
|
||
| F_diff = F_torus(q, theta, radius, core_radius + thickness, nu, | ||
| sld_shell - sld_solvent) - | ||
| F_torus(q, theta, radius, core_radius, nu, sld_shell - sld_core); | ||
|
|
||
| I_total += GAUSS_W[i] * square(F_diff) * sin(theta); | ||
| } | ||
|
|
||
| // multiply by pi/4 to get the integral over [0, pi/2] | ||
| double result = I_total / form_volume(radius, core_radius, thickness, nu) * | ||
| M_PI_4; // A^-1 | ||
|
pkienzle marked this conversation as resolved.
Outdated
|
||
| return 1e8 * result; // convert from A^-1 to cm^-1 | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| r""" | ||
| Definition | ||
| ---------- | ||
|
|
||
| This model computes the scattering from a torus with an elliptical tube | ||
| cross-section and a concentric shell. | ||
|
|
||
| The geometric parameters are: | ||
|
|
||
| * $R$: major (ring) radius of the torus centerline | ||
| * $a$: core minor radius in the radial direction | ||
| * $\nu$: aspect ratio of the elliptical cross-section | ||
| * $t$: shell thickness | ||
|
|
||
| .. figure:: img/torus_elliptical_shell_geometry.png | ||
|
|
||
| Schematic geometry of the torus with elliptical tube cross-section. | ||
|
|
||
| The same aspect ratio $\nu$ is used for both the core and outer cross-sections, | ||
| so the outer semi-axes are $a+t$ and $\nu(a+t)$. | ||
|
|
||
| For a given orientation angle $\theta$ between the torus symmetry axis and | ||
| $\vec q$, the kernel evaluates the amplitude using a numerical integral over | ||
| the tube section: | ||
|
|
||
| .. math:: | ||
|
|
||
| F(q,\theta; x, \Delta\rho) | ||
| = 4\pi\,\Delta\rho\int_{R-x}^{R+x} | ||
| r\,J_0\!\left(qr\sin\theta\right) | ||
| \frac{\sin\!\left(q\,\gamma\cos\theta\right)}{q\cos\theta}\,dr | ||
|
|
||
| with | ||
|
|
||
| .. math:: | ||
|
|
||
| \gamma = \nu\sqrt{x^2-(r-R)^2} | ||
|
|
||
| The core-shell amplitude is formed from outer and inner contributions: | ||
|
|
||
| .. math:: | ||
|
|
||
| F_{cs}(q,\theta) = | ||
| F\!\left(q,\theta;a + t,\rho_{shell}-\rho_{solvent}\right) | ||
| -F\!\left(q,\theta;a,\rho_{shell}-\rho_{core}\right) | ||
|
|
||
| and the orientationally averaged intensity is | ||
|
|
||
| .. math:: | ||
|
|
||
| I(q) = S(q)\int_0^{\pi/2}\!\left|F_{cs}(q,\theta)\right|^2\sin\theta\,d\theta | ||
|
|
||
| where | ||
|
|
||
| .. math:: | ||
|
|
||
| S(q)=1+\frac{\kappa}{1+(q\zeta)^2} | ||
|
|
||
| Here $S(q)$ represents the structure factor accounting for | ||
| interparticle correlations. | ||
|
|
||
| References | ||
| ---------- | ||
| #. T. Kawaguchi, *J. Appl. Crystallogr*, 34(2001) 580-584 | ||
| #. S. Förster, *J. Phys. Chem.*, 103(1999) 6657-6668 | ||
|
|
||
| Authorship and Verification | ||
| --------------------------- | ||
|
|
||
| * **Author:** Itsuki Tajima and Yuhei Yamada (Github user name: Indigo Carmine, https://orcid.org/0009-0003-9780-4135) | ||
| * **Last Modified by:** | ||
| * **Last Reviewed by:** | ||
| """ | ||
|
|
||
| from numpy import inf | ||
|
|
||
| # ---- SasView model metadata ------------------------------------------------- | ||
| name = "torus_elliptical_shell" | ||
| title = "core-shell torus with elliptical cross-section" | ||
| description = "Core-shell torus with elliptical tube cross-section" | ||
| category = "shape:cylinder" | ||
| parameters = [ | ||
| # name units default [min, max] type description | ||
| ["radius", "Ang", 100.0, [0, inf], "volume", "Torus major radius R"], | ||
| [ | ||
| "core_radius", | ||
|
pkienzle marked this conversation as resolved.
Outdated
|
||
| "Ang", | ||
| 5.0, | ||
| [0, inf], | ||
| "volume", | ||
| "Elliptical core minor radius a", | ||
| ], | ||
| ["thickness", "Ang", 2.0, [0, inf], "volume", "Shell thickness"], | ||
| ["nu", "", 1.0, [0.1, 10.0], "volume", "Aspect ratio b/a"], | ||
| ["sld_core", "1e-6/Ang^2", 0.0, [-inf, inf], "sld", "Core SLD"], | ||
| ["sld_shell", "1e-6/Ang^2", 1.0, [-inf, inf], "sld", "Shell SLD"], | ||
| ["sld_solvent", "1e-6/Ang^2", 0.0, [-inf, inf], "sld", "Solvent SLD"], | ||
| ] | ||
|
|
||
| valid = "radius >= core_radius + thickness" | ||
|
|
||
| # -- tell sasmodels that a C kernel is provided ------------------------------- | ||
| source = [ | ||
| "lib/polevl.c", | ||
| "lib/sas_J0.c", | ||
| "lib/gauss76.c", | ||
| "torus_elliptical_shell.c", | ||
| ] # compiled together with default libs | ||
|
pkienzle marked this conversation as resolved.
|
||
|
|
||
|
|
||
| tests = [ | ||
| [{}, 1.002266990452620e-03, 2.390925896904364e07], | ||
| [{}, 2.009078240301904e-03, 2.366687319304617e07], | ||
| [{}, 1.017875911130553e-02, 1.684835184636866e07], | ||
| ] | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you define a separate aspect ratio for thickness,
nu_t, then you should be able to allow tubes with constant thickness usingnu_t = 1. You will need to create a separatenu_outerfor the first term inF_diff, withnu_outer = (nu*core_radius + nu_t*thickness)/(core_radius + thickness).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your suggestion. I agree that this approach would be well suited to micelle-like systems. However, in our system (see the figure below from Ref. 3 by Martin et al.), the aspect ratio appears to be the same throughout. Therefore, we would prefer to keep a single parameter, ν, for fitting.
Would you have any suggestions for how we might improve the model under this assumption?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether we have a model with independent or shared aspect ratios depends on the community. If they are independent you can constrain them to be shared for your use case, but it is more work for you. If they are shared, then anyone wanting them independent will have to implement their own version of the model.
Without dispersion in nu, you can tie M1.nu_t = M1.nu on the constrained fitting page in sasview. With dispersion this will integrate over the outer product nu × nu_t, which is not what you want. Instead you need to reparameterize the model with nu_t = nu (docs here) so that you only have dispersion in shared_nu. The following should work:
You could perhaps simplify it to the following so that you only have the nu parameter, but I haven't tested:
A question for the broader sasmodels/sasview community: do we want many variations on the same model in the sasmodels library, or do we want one base version with different variants selected via constraints (#215). For example, you can parameterize an ellipsoid by some combination of volume, aspect ratio, polar radius and equatorial radius. For a core-shell ellipsoid, you could further choose a hollow variant (with core sld implicitly equal to solvent sld, and volume fraction calculated from only the material in the shell) and a filled variant (where volume fraction includes the enclosed volume even when core sld is constrained to equal solvent sld).
I think the best answer is to build variants into the model infrastructure so that the underlying kernel code is shared across all variants. This will require some additional work in the sasview model selector so that the different parameterizations can be selected.
Meanwhile, I'll leave it to others to comment on which variant(s) of toroidal models should be supported in sasmodels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand your recommendation. Adding many variations to sasmodels might make the implementation less clear. I’ve implemented the changes as suggested for now. If using variants in SasView is preferred, I’d be happy to help with that—please feel free to @mention me.