atcollab / at

Accelerator Toolbox
Apache License 2.0
48 stars 31 forks source link

Beam slice monitor #692

Closed swhite2401 closed 7 months ago

swhite2401 commented 8 months ago

This PR introduces a new diagnostic allowing to monitor the bunches slices mean and std. This is useful for collective ffects simulations.

swhite2401 commented 8 months ago

@lcarver here is the long awaited slice monitor, delivered just in time. It is not perfect but allows for testing. I think in the final version once should have a separate attribute for zpos and not calculate zstd which makes no sense. Comments welcome!

lcarver commented 7 months ago

Hello, it doesn't work sorry.

sm = at.SliceMoments('slicer', 200, startturn=0, endturn=1000) or

sm = at.SliceMoments('slicer', nslice=200, startturn=0, endturn=1000)
fring.append(sm)

print(fring[-1]) gives TypeError: __init__() missing 1 required positional argument: 'nslice'

lcarver commented 7 months ago

I made a work around, you may commit it if you wish, but just to skip this error.

def __init__(self, family_name: str, **kwargs):
    kwargs.setdefault('PassMethod', 'SliceMomentsPass')
    self.nslice = kwargs.pop('nslice', 100)
    self._stds = numpy.zeros((4, self.nslice, 1), order='F')
    self._means = numpy.zeros((4, self.nslice, 1), order='F')
    self._weights = numpy.zeros((self.nslice, 1), order='F')
    self._startturn = kwargs.pop('startturn', 0)
    self._endturn = kwargs.pop('endturn', 1)
    super(SliceMoments, self).__init__(family_name, **kwargs)
    self.startturn = self._startturn
    self.endturn = self._endturn
    self.set_buffers(self._endturn, 1)
lcarver commented 7 months ago

I think the output is not set correctly.

For nslice=32, nbunch=16, nturns=1000

sm = at.SliceMoments('slicey_boi', nslice=32, startturn=0,endturn=1000)
print(sm.means.shape)

output (4,512,1)

EDIT; endturn is doing some weird things.

print(sm.endturn)
1

EDIT2: the problem is of course the fact that I come out of the loop each time. my bad.

lcarver commented 7 months ago

My main comment at the moment is that I would like to have the output be of the form

sm.means.shape (4, nbunch, nslice, nturn)

it is easy to get the bunches out from it, but it is an extra step that I think it is better if it is not needed

lcarver commented 7 months ago

To summarise my tests:

SliceMoments works very well. I see almost perfect agreement with a np.histogram of the last turn. Not tested yet with MPI. However there are some small features that would be nice to include:

Change shape to (4, nbunch, nslice, nturn) instead of (4,nbunch*nslice, nturn). The former is much nicer to use in post processing).

At the moment if i want current distribution I have to do plt.plot(sm.means[4,:sm.nslice], sm.weights[:sm.nslice]) (for bunch 0 for example), but if the weight of the slice is 0, which it often is at the tails, then the mean value is also 0 and you get a mess in the plot.

image

image

lcarver commented 7 months ago

Longitudinal distributions without MPI Here we are in 4bunch mode with 4 different bunch currents. Last turn only. image

lcarver commented 7 months ago

Same simulation, with MPI 40 threads image

swhite2401 commented 7 months ago

Hello, it doesn't work sorry.

sm = at.SliceMoments('slicer', 200, startturn=0, endturn=1000) or

sm = at.SliceMoments('slicer', nslice=200, startturn=0, endturn=1000)
fring.append(sm)

print(fring[-1]) gives TypeError: __init__() missing 1 required positional argument: 'nslice'

I cannot reproduce this error, the correct syntax is:

fring, fringr = at.fast_ring(ring)
bpm = at.SliceMoments('BPM', 3, endturn=2)
fringr.append(bpm)
print(bpm)

nslice needs to be a positionnal argument because this uses memory and I want that the user knows what he is doing.

swhite2401 commented 7 months ago

Fix problem with mean zposition being 0 for weight = 0.

zposition now always has a value (center of slice in case weight=0), other values are NaNs in case weight=0

swhite2401 commented 7 months ago

Change shape of output

The shape of the attributes was changed according to recommendations

swhite2401 commented 7 months ago

I have added the spos attribute, please check the output

lcarver commented 7 months ago

Hello, it doesn't work sorry. sm = at.SliceMoments('slicer', 200, startturn=0, endturn=1000) or

sm = at.SliceMoments('slicer', nslice=200, startturn=0, endturn=1000)
fring.append(sm)

print(fring[-1]) gives TypeError: __init__() missing 1 required positional argument: 'nslice'

I cannot reproduce this error, the correct syntax is:

fring, fringr = at.fast_ring(ring)
bpm = at.SliceMoments('BPM', 3, endturn=2)
fringr.append(bpm)
print(bpm)

nslice needs to be a positionnal argument because this uses memory and I want that the user knows what he is doing.

In your commit ' add nslice to build attributes ' the problem is fixed. However, for sure the earlier commits were not working and I could reproduce it by commenting out the build attributes addition, but who cares. It now works thanks to this commit. Maybe you had it included but not committed. But it doesn't matter. It works now.

lcarver commented 7 months ago

Hello, I run a simulation with 8 bunches, each current is different (linearly increasing) and symmetrically placed. I look at the distribution with only one turn and initialised with nslice=200

print(sm.weights.shape) (8, 200, 1)

plt.plot(sm.weights[0,:,0]) image

so we have a problem with the assignment of the data in the new format.

lcarver commented 7 months ago

I guess the new way of plotting charge distribution is with

plt.plot(sm.spos[0,:,0], sm.weights[0,:,0])

for bunch 0 and turn 0 of the slice monitor. The nans are there and seem to be doing their job correctly, but because the wrong data is assigned to the shape it is hard to make a nice conclusion.

swhite2401 commented 7 months ago

@lcarver , is this what you were expecting?

lcarver commented 7 months ago

The output is good. Same case as before

No MPI: image

With MPI: image

One last small remark: why does the s position not agree exactly? image

lcarver commented 7 months ago

I tried with varying beam sizes and it works as expected. Just waiting on the change for the spos and then its fine.

lcarver commented 7 months ago

Works as expected. Thanks a lot.

swhite2401 commented 7 months ago

Sorry @lcarver , I have made some changes to document the functions and handle the zero current case, it will need to be re-validated/approved

lcarver commented 7 months ago

Everything works perfectly.