Opened 4 years ago
Closed 3 years ago
#836 closed defect (fixed)
orient_symmetric in new coordinates
Reported by: | richardh | Owned by: | richardh |
---|---|---|---|
Priority: | minor | Milestone: | SasView 4.1.0 |
Component: | SasView | Keywords: | |
Cc: | Work Package: | SasView Bug Fixing |
Description
Following my email to the developers list, I suspect the equation used in kernel_header.c, for function orient_symmetric may be wrong for the new coordinates.
It has
cos(alpha) =(cosPhi.qx + sinPhi.qy)cosTheta/q
Subject to confirmation, I think after some horrible cosine rule geometry it should be
(sinTheta.qy + cosTheta.cosPhi.qx)/q
This needs to be tested & checked, as I may have misunderstood something.
Richard
Change History (6)
comment:1 Changed 4 years ago by richardh
comment:2 Changed 4 years ago by richardh
[turned out I was intially verifying the old coordinate transform,
which has the coaAlpha = (sinTheta.qy + cosTheta.cosPhi.qx)/q ]
comment:3 Changed 4 years ago by richardh
Seems that cos(alpha) =(cosPhi.qx + sinPhi.qy)cosTheta/q
should have been cos(alpha) =(cosPhi.qx + sinPhi.qy)sinTheta/q
have fixed this, now get circular symmetry 2d I(qx,qy) when theta=0 as expected.
The original 2d unit tests now work when the appropriate new values of theta and phi are applied, and so have been re-instated, in cylinder and core_shell_ellipsoid. [We are still woefully short of 2d unit tests in general.]
Does this have any knock on consequences in any magnetic models?
I am not closing the ticket yet, as we need to look at ORIENT_ASYMMETRIC used by the elliptical cylinder etc, this only has one version, but perhaps it is generic?
comment:4 Changed 4 years ago by richardh
Ahah - the new definitionas of theta and phi are so far ONLY implemented for the axially symmetric particles such as cylinder, the asymmetric ones such as elliptical cylinder and parallelepiped are still using the original coordinates.
The diagrams in the documentation are correct in this respect.
I will downgrade the severity of this ticket, pending comments from anyone else.
[I do need to debug the 2d calculation from my new core_shell_bicelle_elliptical model, which currently looks wrong. Will add a ticket for that.]
comment:5 Changed 4 years ago by richardh
- Priority changed from critical to minor
comment:6 Changed 3 years ago by richardh
- Resolution set to fixed
- Status changed from new to closed
Have now found #776, of which this ticket is a subset, which was put in to fix a bug in orient_symmetric.
Following discussion at developers meeting 7th Feb 2017, we will leave version 4.1 with new coordinates for the symmetric particles but not the asymmetric ones, pending an outcome to the discussions in #776
For the new coordinate system, I think the final cosTheta should actually be sinTheta,
will test tomorrow,
Richard