Skip to content
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

Isotropic vector generation #9

Open
nkx111 opened this issue May 11, 2021 · 12 comments
Open

Isotropic vector generation #9

nkx111 opened this issue May 11, 2021 · 12 comments
Assignees
Labels
development enhancement New feature or request

Comments

@nkx111
Copy link
Member

nkx111 commented May 11, 2021

G4ThreeVector PrimaryGeneratorAction::GetIsotropicVector() {

Generating vector in this way may not be isotropic. Shouldn't we use θ and Φ to randomize direction?

@jgalan
Copy link
Member

jgalan commented May 11, 2021

I believe the present implementation should be isotropic. Using angles would be more complex implementation

Here is a good example
http://corysimon.github.io/articles/uniformdistn-on-sphere/

Do you have any clues pointing to a problem on isotropy with present vector?

@jgalan
Copy link
Member

jgalan commented May 11, 2021

Our present implementation is closer to the "Alternative method 1", however, we do not protect normalization by very small vectors, as they do, this could cause problems for us:

They require that the norm is larger than 0.001, we could improve that in our method.

@lobis
Copy link
Member

lobis commented May 11, 2021

I think we should really use the internal features of geant4 for this, using the G4GeneralParticleSource. it will also work with the random seed internally. I will update this function as an example to see the difference it makes, I think it will greatly simplify our code.

We can only use the angular distribution from the generator, we do not need to change from G4ParticleGun.

I will do this later this morning.

@lobis lobis added development enhancement New feature or request labels May 11, 2021
@nkx111
Copy link
Member Author

nkx111 commented May 11, 2021

The currecnt method is by throwing points randomly in a cube, and calculate their directions. I guess there will be more events near the 8 corners of the cube

@lobis lobis added the bug Something isn't working label May 11, 2021
@nkx111
Copy link
Member Author

nkx111 commented May 11, 2021

OK, I see we rejected the events with n >1. Then this is right. Isotropic indeed. Just be less elegant.

@jgalan
Copy link
Member

jgalan commented May 11, 2021

Yes, you always get a uniform distribution within a sphere of radius 1. Indeed, efficiency goes with 4/3 * PI * (D/2)^3/D^3. Volume of the sphere versus volume of the cube.

However, the calculation of cosines and other mathematical functions required in other methods believe is more expensive computationally.

@jgalan jgalan removed the bug Something isn't working label May 11, 2021
@jgalan
Copy link
Member

jgalan commented May 11, 2021

Ok, finally I believe there is a bug there, because the random number is running between -0.5 and 0.5, so the radius is 0.5 and not 1!

@jgalan
Copy link
Member

jgalan commented May 11, 2021

No no, it is ok, it is ok, it is divided by 0.5 so it is runnning between -1 and 1. :)

@lobis
Copy link
Member

lobis commented May 11, 2021

This generator is improved in commit (3826fd1) of the branch lobis-patch. There you can see this new approach, which I think is much better.

I will do the pull request once it is better organized, feel free to discuss the approach, I think it could be ported to all distributions we are currently using.

@jgalan
Copy link
Member

jgalan commented May 11, 2021

It is probably more time consuming than the simple implementation we had. On top of that it loses transparency, before we were at least able to go to the method and read clearly what it was doing.

@jgalan
Copy link
Member

jgalan commented May 11, 2021

void G4SPSAngDistribution::GenerateIsotropicFlux(G4ParticleMomentum& mom)
  311 {
  312   // generates isotropic flux.
  313   // No vectors are needed.
  314   G4double rndm, rndm2;
  315   G4double px, py, pz;
  316 
  317   //
  318   G4double sintheta, sinphi,costheta,cosphi;
  319   rndm = angRndm->GenRandTheta();
  320   costheta = std::cos(MinTheta) - rndm * (std::cos(MinTheta) - std::cos(MaxTheta));
  321   sintheta = std::sqrt(1. - costheta*costheta);
  322   
  323   rndm2 = angRndm->GenRandPhi();
  324   Phi = MinPhi + (MaxPhi - MinPhi) * rndm2; 
  325   sinphi = std::sin(Phi);
  326   cosphi = std::cos(Phi);
  327 
  328   px = -sintheta * cosphi;
  329   py = -sintheta * sinphi;
  330   pz = -costheta;
  331 
  332   // for volume and ponit source use mother or user defined co-ordinates
  333   // for plane and surface source user surface-normal or userdefined co-ordinates
  334   //
  335   G4double finx, finy, finz;
  336   if (posDist->GetSourcePosType() == "Point" || posDist->GetSourcePosType() == "Volume") {
  337     if (UserAngRef){
  338       // Apply Rotation Matrix
  339       // x * AngRef1, y * AngRef2 and z * AngRef3
  340       finx = (px * AngRef1.x()) + (py * AngRef2.x()) + (pz * AngRef3.x());
  341       finy = (px * AngRef1.y()) + (py * AngRef2.y()) + (pz * AngRef3.y());
  342       finz = (px * AngRef1.z()) + (py * AngRef2.z()) + (pz * AngRef3.z());
  343     } else {
  344       finx = px;
  345       finy = py;
  346       finz = pz;
  347     }
  348   } else {    // for plane and surface source   
  349     if (UserAngRef){
  350       // Apply Rotation Matrix
  351       // x * AngRef1, y * AngRef2 and z * AngRef3
  352       finx = (px * AngRef1.x()) + (py * AngRef2.x()) + (pz * AngRef3.x());
  353       finy = (px * AngRef1.y()) + (py * AngRef2.y()) + (pz * AngRef3.y());
  354       finz = (px * AngRef1.z()) + (py * AngRef2.z()) + (pz * AngRef3.z());
  355     } else {
  356       finx = (px*posDist->GetSideRefVec1().x()) + (py*posDist->GetSideRefVec2().x()) + (pz*posDist->GetSideRefVec3().x());
  357       finy = (px*posDist->GetSideRefVec1().y()) + (py*posDist->GetSideRefVec2().y()) + (pz*posDist->GetSideRefVec3().y());
  358       finz = (px*posDist->GetSideRefVec1().z()) + (py*posDist->GetSideRefVec2().z()) + (pz*posDist->GetSideRefVec3().z());
  359     }
  360   }
  361   G4double ResMag = std::sqrt((finx*finx) + (finy*finy) + (finz*finz));
  362   finx = finx/ResMag;
  363   finy = finy/ResMag;
  364   finz = finz/ResMag;
  365 
  366   mom.setX(finx);
  367   mom.setY(finy);
  368   mom.setZ(finz);
  369 
  370   // particle_momentum_direction now holds unit momentum vector.
  371   if(verbosityLevel >= 1)
  372     G4cout << "Generating isotropic vector: " << mom << G4endl;
  373 }

@lobis
Copy link
Member

lobis commented May 13, 2021

It is probably more time consuming than the simple implementation we had. On top of that it loses transparency, before we were at least able to go to the method and read clearly what it was doing.

Well, I don't think the performance is a bottleneck here, as the time generating primaries is most of the time neglectable compared to the tracking. Maybe in the worst case scenario you could save a few minutes.

Regarding the "transparency", I guess this is a matter of opinion. I am of the opinion that the less code, the better, as more code means more maintenance. Personally I think the tradeoff if justified here, but it is just an opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants