Changes in sroptcryst.h to add Laue geometry#19
Open
Angelrf86 wants to merge 1 commit intoochubar:masterfrom
Open
Changes in sroptcryst.h to add Laue geometry#19Angelrf86 wants to merge 1 commit intoochubar:masterfrom
Angelrf86 wants to merge 1 commit intoochubar:masterfrom
Conversation
ochubar
reviewed
Oct 2, 2019
| //TVector3d m_nv; // horizontal, vertical and longitudinal coordinates of outward normal to crystal surface in the frame of incident beam | ||
| TVector3d m_tv; // horizontal, vertical and longitudinal coordinates of central tangential vector [m] in the frame of incident beam | ||
| TVector3d m_sv; // horizontal, vertical and longitudinal coordinates of central saggital vector [m] in the frame of incident beam | ||
| TVector3d v_nv; //* horizontal, vertical and longitudinal coordinates of outward normal to crystal surface in the frame of incident beam */ //ARF06062019 Introduce Vector3d& v_nv, |
Owner
There was a problem hiding this comment.
It's not clear why this (redundant) vector is re-introduced; it seems to be still used only in ctor (srTOptCryst::srTOptCryst). I reject this for the moment.
| int m_uc; // crystal use case: 1- Bragg Reflection, 2- Bragg Transmission | ||
| // case: 3- Laue, 4- Laue Transmission ARF08052019 change char to | ||
|
|
||
| int m_ug; //ARF2305219 This variable define Laue (2) or Bragg (1) |
Owner
There was a problem hiding this comment.
Do we really need it, if m_uc now clearly identifies each of the 4 supported cases. I reject this for the moment.
| TVector3d v_nv; //* horizontal, vertical and longitudinal coordinates of outward normal to crystal surface in the frame of incident beam */ //ARF06062019 Introduce Vector3d& v_nv, | ||
|
|
||
| int m_uc; // crystal use case: 1- Bragg Reflection, 2- Bragg Transmission | ||
| // case: 3- Laue, 4- Laue Transmission ARF08052019 change char to |
Owner
There was a problem hiding this comment.
Why changing char to int (char can have values 1 to 4)? I reject for the moment.
| double m_PolTrn[2][2]; // 2x2 transformation matrix of the polarizations from (e1X,e2X) to (sg0X,pi0X). | ||
| double m_InvPolTrn[2][2]; //OC06092016 | ||
| double m_HXAi[3]; // Reciprocal lattice vector coordinates | ||
| double m_HXAi_bee[3]; //ARF03062019 Reciprocal lattice vector coordinates in beam coordinates |
| double m_sg0X[3]; | ||
| double m_cos2t; //cos(2.*thBrd); | ||
|
|
||
| double thBrd; //ARF23052019 to calculate the gamma0 and gammaH |
Owner
There was a problem hiding this comment.
Not sure why double thBrd, alphrd need to be member variables; they seem to be used only in ctor. I reject for the moment.
| double uRL = sqrt(m_RLabXt[0][2] * m_RLabXt[0][2] + m_RLabXt[1][2] * m_RLabXt[1][2] + m_RLabXt[2][2] * m_RLabXt[2][2]); | ||
| const double pi = 4.*atan(1.); | ||
| double thBrd = acos(uBrd / uHX / uRL) - pi / 2.; | ||
|
|
| m_HXAi_bee[0] = 0; //ARF03062019 Change definition for the calculation of bee | ||
| m_HXAi_bee[1] = - sin(thBrd) / m_dA; //IP14062019 for Laue pi/2 rotation of rec. vector from Bragg | ||
| m_HXAi_bee[2] = - cos(thBrd) / m_dA; //IP14062019 | ||
| } |
| } | ||
|
|
||
| void FindDefOutFrameVect(double phEn, double hn, double ht, TVector3d& tv, TVector3d& sv, TVector3d& vZ1, TVector3d& vX1) | ||
| void FindDefOutFrameVect(double phEn, double hn, double ht, TVector3d& nv, TVector3d& tv, TVector3d& sv, TVector3d& vZ1, TVector3d& vX1) //ARF06062019 Introduce Vector3d& nv, |
| // defined by Zachariasen gamma0, b, alpha and z.) | ||
| double gamma0 = -u0X[1]; | ||
| double bee = 1. / (1. + m_HXAi[1] / k0wXAi[1]); | ||
| double bee = 1. / (1. + (m_HXAi_bee[1] * m_RXtLab[1][1] + m_HXAi_bee[2] * m_RXtLab[2][1]) / k0wXAi[1]); //ARF03062019 change from: double bee = 1. / (1. + m_HXAi[1] / k0wXAi[1]); |
| Cph2C = exp(ph2C); | ||
| D0trsC = -Cph2C*(x2C - x1C)/x1C; | ||
| } | ||
| else if(real(ph2C) > logDBMax) |
Owner
There was a problem hiding this comment.
These modifications look scary to me, as you remove special processing of possible overflow cases.
I have to compare these changes with John's code for the Laue case before proceeding.
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
2 participants
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.
Dear all,
We have been working in adding the Laue Case to SRW. This is the merge request after having cross check that the simulations perform with the Laue code fit the theory and comparison with other codes.
Best wishes,
Angel, Ilia, Liubov