You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Your difficultly may be with an unrelated bug in the spinner controls that I just reported as issue #36 . However, here is a current method for reproducing the "broken Bragg spots".
Start the viewer. Click in the Domain size text box, type 8000, and hit "return". Click in the Mosaic angle text box, type 1, and hit "return". Now you should see Bragg spots that look like multiple bright pixels.
I can reproduce this - is an entry box controlling "number of mosaicity samples" an acceptable solution for the short term ? I would say we need another one for "number of spectrum energy samples"
On Tue, May 16, 2023 at 2:44 PM Derek Mendez ***@***.***> wrote:
I can reproduce this - is a box controlling " number of mosaicity samples"
acceptable solution for the short term ? I would say we need another one
for "number of spectrum energy samples"
—
Reply to this email directly, view it on GitHub
<#25 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADQ24VXFSEGM7G5FVWVQCI3XGPYNTANCNFSM6AAAAAAX3UGLKM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
@dermen I reverted the (apparently wrong) conversion to radians. We have a hyperparam now for number of mosaic domains as well as one for oversampling. Can you try those out and see if adjusting these can smooth the broken spots?
It's not yet clear to me which of these cases we're in:
math/science/calcs are factually wrong
simulation isn't visually representative of reality due to limitations of sampling
simulation isn't visually representative of reality due to poor choice of default params
TL;DR I have concluded this issue partly arises from mis-using nanobragg (at least the given reproducer). Its impractical to allow users to simulate huge mosaic blocks. I don't have a specific solution, but I have some design choices below that make most sense based on my use of nanoBragg (and a desire to add background to the images).. But as it stands, the quickest way to "solve" this issue is adding an oversample and number_of_energy_channels field for users , and beyond this we also should impose some threshold parameters (e.g. maximum domain size for a given oversample, maximum bandwidth for a given "number of energy channels").
===================================
This code:
defon_update_domain_size(self, skip_gen_image_data=False):
"""given a target domain size (one side length), determine number of cells to use along each axis"""side_length=self.params_num.domain_size.get_value()
a, b, c, _, _, _=self.scaled_ucellself.ncells= []
forsidein (a, b, c):
self.ncells.append(max(int(round(side_length/side)), 1))
ifnotskip_gen_image_data:
self._generate_image_data()
updates Ncells_abc when domain size is increased.. However this is dangerous, as we know nanoBragg can only handle Ncells_abc up to certain values (and small values). Simulating e.g. a 200x200x200 mosaic block will indeed require a large amount of oversample.
When I start the GUI, self.ncells is 13x26x26 (maximum pixel intensity = 9.65e-6), however after toggling the mosaic domain size to 9000 (from 1000), ncells becomes 114x114x234 (maximum pixel intensity=0.017). Note, these pixel intensities are way too small.. If I toggle "Use structure factors" , then things get better (0.33 max intensity for 1000 domain size, 421 max intensity for 9000 domain size), but that's an aside for this issue.
I propose two solutions that will help fix this "issue" (the "issue" arises from mis-using nanoBragg as its currently written, and we need to guard against this).
a) allow the user to enter 3 numbers (Na, Nb, Nc). This is important, because the aspect ratios of the mosaic block influence the positions of the spots (think spherical block versus skinny ellipsoidal block). So users will provide Na,Nb,Nc (we will compute a "mosaic block size" automatically, and display it to the user). Then, user will provide a "crystal size " parameter, which will compute a volume scale factor to bring the scattering up to physical units (so we can add a physical background)..
b) user provides two "size" parameters, one for block size (as it is now), and another for crystal size. the former will have a restriction which we pre-compute ahead of time (e.g. max block size determined from a 90x90x90 block or something dependent on the unit cell). Similar to part a), the crystal size will be used to compute a scale factor to bring the scattering up to physical units..
Otherwise, the user will never know when they are entering the out-of-scope region (too large of a mosaic block).
Now we can always increase oversample, but things can also easily get out-of-hand and impractical. We just need to make this clear to the user in order to fix this issue..
Also, as an aside, we need to scale the self.SIM_noSF model to have the same "structure factor intensity" as the self.SIM model.. This will fix the issue above that default SIM has really small (1e-6) pixel intensities..
Activity
dermen commentedon May 16, 2023
What are the commands needed to reproduce the issue: WHen I do
I cant seem to increase the Gaussian spectrum bandwidth beyond 2%, and I don't see broken spots at that bandwidth...
nksauter commentedon May 16, 2023
Your difficultly may be with an unrelated bug in the spinner controls that I just reported as issue #36 . However, here is a current method for reproducing the "broken Bragg spots".
Start the viewer. Click in the Domain size text box, type 8000, and hit "return". Click in the Mosaic angle text box, type 1, and hit "return". Now you should see Bragg spots that look like multiple bright pixels.
dermen commentedon May 16, 2023
I can reproduce this - is an entry box controlling "number of mosaicity samples" an acceptable solution for the short term ? I would say we need another one for "number of spectrum energy samples"
nksauter commentedon May 16, 2023
dermen commentedon May 23, 2023
Hey all, I wasnt sure that radians was in fact the issue - was this verified , other than visually ?
dermen commentedon May 23, 2023
it seems like the generate_umats method actually accepts a number in degrees...
irisdyoung commentedon May 23, 2023
@dermen I reverted the (apparently wrong) conversion to radians. We have a hyperparam now for number of mosaic domains as well as one for oversampling. Can you try those out and see if adjusting these can smooth the broken spots?
It's not yet clear to me which of these cases we're in:
Still need help sorting this one out.
dermen commentedon May 23, 2023
TL;DR I have concluded this issue partly arises from mis-using nanobragg (at least the given reproducer). Its impractical to allow users to simulate huge mosaic blocks. I don't have a specific solution, but I have some design choices below that make most sense based on my use of nanoBragg (and a desire to add background to the images).. But as it stands, the quickest way to "solve" this issue is adding an oversample and number_of_energy_channels field for users , and beyond this we also should impose some threshold parameters (e.g. maximum domain size for a given oversample, maximum bandwidth for a given "number of energy channels").
===================================
This code:
updates Ncells_abc when domain size is increased.. However this is dangerous, as we know nanoBragg can only handle Ncells_abc up to certain values (and small values). Simulating e.g. a 200x200x200 mosaic block will indeed require a large amount of oversample.
When I start the GUI, self.ncells is 13x26x26 (maximum pixel intensity = 9.65e-6), however after toggling the mosaic domain size to 9000 (from 1000), ncells becomes 114x114x234 (maximum pixel intensity=0.017). Note, these pixel intensities are way too small.. If I toggle "Use structure factors" , then things get better (0.33 max intensity for 1000 domain size, 421 max intensity for 9000 domain size), but that's an aside for this issue.
I propose two solutions that will help fix this "issue" (the "issue" arises from mis-using nanoBragg as its currently written, and we need to guard against this).
a) allow the user to enter 3 numbers (Na, Nb, Nc). This is important, because the aspect ratios of the mosaic block influence the positions of the spots (think spherical block versus skinny ellipsoidal block). So users will provide Na,Nb,Nc (we will compute a "mosaic block size" automatically, and display it to the user). Then, user will provide a "crystal size " parameter, which will compute a volume scale factor to bring the scattering up to physical units (so we can add a physical background)..
b) user provides two "size" parameters, one for block size (as it is now), and another for crystal size. the former will have a restriction which we pre-compute ahead of time (e.g. max block size determined from a 90x90x90 block or something dependent on the unit cell). Similar to part a), the crystal size will be used to compute a scale factor to bring the scattering up to physical units..
Otherwise, the user will never know when they are entering the out-of-scope region (too large of a mosaic block).
Now we can always increase oversample, but things can also easily get out-of-hand and impractical. We just need to make this clear to the user in order to fix this issue..
Also, as an aside, we need to scale the self.SIM_noSF model to have the same "structure factor intensity" as the self.SIM model.. This will fix the issue above that default SIM has really small (1e-6) pixel intensities..