-
Notifications
You must be signed in to change notification settings - Fork 119
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
Gauss star gonio #985
base: master
Are you sure you want to change the base?
Gauss star gonio #985
Conversation
* the tests renamed to simtbx/tests/tst_api_congruency and also checks the simtbx/kokkos/simulations kernel
I'd like to make sure that the pure-kokkos version of exascale_api supports this new feature. Probably need a short call about this. |
You are saying that "a perfectly round RELP called gauss_star [...] eliminates issues where F_latt varies with symmetry operations". Which is great, because it should fix C- and F- centered cells; however, this PR changes the way Na, NB, and Nc, numbers of unit cells in microcrystal, are used, doesn't it? Does it mean the new behaviour is to 1) take Na, Nb, Nc lattice parameters in each direction in the primitive cell as before or 2) the phil-supplied (i.e. primitive or non-primitive) cell and somehow scale intensities if necessary? |
@nksauter definitely happy to meet this week- I think it still needs some testing, not quite ready to merge it in yet.. There is a new test called @Baharis , the new model is limited compared to the original gaussian shape. We can discuss.. But I believe Na,Nb,Nc are meant to correspond to numbers of cells along the primitive lattice. I need to add a change to diffBragg_cpu_kernel.cpp to reflect this.. |
Ok, if that is the case then I unfortunately do not understand the problem this PR is solving. Apparently, it solves issues with simulating some space groups, but as long as Na,Nb,Nc correspond to numbers of cell along the primitive lattice, I- and F-centered cells will be always simulated wrong. As for the "full support of goniometer rotation in diffBragg", does it mean diffBragg can be now used to simulate rotation data in addition to stills? |
@Baharis, to elaborate, the new model only depends on the product of NaNbNc , so it might not be the most realistic.. |
and yes, adding the gonio allows to refine rotation data, however the current code does not allow refinement of the goniometer axis .. though that would be nice to test |
I was confused before but now I am completely lost. Being away for the last month I probably missed some crucial information here, so maybe I'd better just refrain from unjustified comments. I'd gladly discuss this on some meeting if my input is appreciated. |
@Baharis , I know you were also working on a fix for the body/face centered cells - you could use the script E.g. you can try
and it will pass, however if you use the square RELPS, it will fail:
We made the fix for ExaFEL by letting F_latt be a "sum of F_latts", and you can test that here:
however, as you pointed out, that will fail for I/F, e.g.
But the gaussian star model allows this last test to pass
You can use |
I will note, I did some tests and now think the coefficient in the exponential is slightly different (I think closer to 1.75 as opposed to 1.9). Thats why the fudge is there :) |
* run_tests runs tst_api_congruency now
This includes two big additions to diffBragg and modeling
The Delta-Q falloff was suggested by James here, which fixed the issue we were observing with F_latt for some space groups.. however still in the process of determining exactly what goes in the exponential..