Fill pattern

[swhite2401]

Beam characteristics are added to the Lattice object:

• fillpattern: contain information on the filled buckets and relative amplitude
• bunch_spos: bunch spatial distribution
• bunch_currents: bunch current distribution
• nbunch: number of bunches
• beam_current: total beam current (mean to replace the current properties in all collective effects passmethods)

These attributes are then accessible to the c passmethods. This is a pre-requisit for the implementation of multi-bunch tracking implementation that will be introduced in a future PR.

Existing pyAT functionalities are not modified by this PR.

[lfarv]

Hi @swhite2401. As you suspected, I think that duplicating the harmonic number in the Particle object is a very bad thing. But I am not sure that this is necessary. If I look at at.c, I see that you just need:

• a total current,
• a number of bunches,
• an array of "weights" as long as the bunch number.

I understand that the bunch are equally spaced. And the harmonic number does not appear anywhere.

So why do you need more in the Particle object? Store the same information (even the number of bunches is redondant: the length of the "weight" array is enough).

The only point is to check the validity of the number of bunches. So let the Particle constructor create a Particle with 0 current and 0 bunch.

To set bunches, you must call a method to which you provide the weight array, and the lattice. Then you can check that the Lattice.harmonic_number is a multiple of the number of bunches.

Or, may be better, you add a method to the Lattice object, which will set the bunches of its Particle attribute, through a private method of the Particle. A standalone Particle cannot have bunches, but that makes sense.

Well, may be I missed something, but that's an idea…

[swhite2401]

I understand that the bunch are equally spaced. And the harmonic number does not appear anywhere.

No bunch distribution can be arbitrary, in which case a vector (harmn,) is needed and the bunch spacing is required for long range wake field calculations, I'll look into integration some function into the lattice_object. This is what I started with but I thought this was already overloaded...but you are right it is cleaner

[lfarv]

Hi @swhite2401: I was not clear enough in my description. I think that you can efficiently describe any arbitrary bunch combination with 2 variables;

1. the total current (float)
2. an array of floats (weights, that I would rather call filling_pattern), such that the harmonic number is a multiple of its length. It defines equidistant buckets, and the values are the fraction of the total current: 0.0 means empty bucket. The most general case is when the length of the array is equal to the harmonic number, where non-zero values indicate filled bunches. But shorter arrays may define common "few bunch" distributions.

So the harmonic number is only necessary in the property setter to check the length of the filling_pattern array. As you proposed, a scalar value could be interpreted as a number of equidistant and equally filled bunches.

It at.c, I see;

struct parameters
{
  ...
  double beam_current;
  double nbunch;
  double bunch_spacing;
  double *bunch_currents;
};

These 2 attributes are sufficient to fill beam_current, nbunch, bunch_current. bunch_spacing is computed in C as param.bunch_spacing = param.RingLength/param.nbunch;, so if these variables are sufficient for the multi-bunch tracking, you don't need more than these two properties.

Now I think that total_current and filling_pattern could be properties of the Lattice object rather that Particle. This is simpler, total_current can be a normal attribute, filling_pattern must be a property, and the property setter can also take care of normalisation so that sum(filling_pattern) = 1.0. It's also possible that it converts a short array into a full (nharm,) array, if it makes further processing easier.

This means a very small modification of the original lattice_object.py:

• in the constructor, initialise total_current at 0 and filling_pattern as empty array,
• a property setter for the filling_pattern array.
• preferably resetting at empty filling_pattern when harmonic_number is changed.

Almost nothing to modify in at.c compared to your version: take properties from the lattice rather than particle.

I have a question concerning the future tracking: do you intend to track a single macro-particle in each bunch ? Then how to ensure that r_in is consistent with the declared filling_pattern (r_in particles in the right bunch)?

[swhite2401]

@lfarv, ok I get the point you mean param.bunch_spacing = param.RingLength/nbuckets; where nbuckets is the length of the fillingpattern vector, this would work yes but it really doesn't make any difference.
I have considered what you proposed, i.e. a single attribute for everything, but I found that separating the weights and the empty/filled buckets array was more user friendly.
Of course internally this can be reduced to a single array passed to at.c, I'll look into it.

It ok for me to move everything to the Lattice object, but I had in mind to start to introduce a beam object that we could use for tracking which I think would be a very useful development...anyway I'll do as you suggest we can keep this for later

[swhite2401]

Now concerning your question it is a bit complicated, but I have a solution to handle multiple particles per bunch.
I am happy to discuss anytime and curious to have your opinion it will be more efficient than on git.

[swhite2401]

Hello @swhite2401 For the Lattice object, this seems to work. I'll put anyway a few comments:

* By using 4 attributes (`_beam_current`, `_fillpattern`, `_bunch_spos`, `_bunch_currents`), you are storing a lot of redundant information. In addition to this inelegant design of the lattice object, this forces to introduce correlations: when you modify one, you need to update some others. This is complicated and error prone.

* On the other hand, by squeezing the fill pattern to keep only the filled bunches, you "almost" loose the bunch number information (yes, you could get it back indirectly with `_bunch_spos`…). These may be useful.

A simpler solution is to keep only 2 attributes:

1. `beam_current`, as a normal attribute instead of a property,

2. the "non-squeezed" normalised fill pattern, as property + private attribute

These two contain the full information, and they are orthogonal: modifying one does nor influence the other. They represent exactly what you defined as the public interface in the present design. They are also the only ones needed in a copy.

This results in a much cleaner lattice object. Of course, you can still offer a number of read-only properties for informational purpose: number of bunches, bunch numbers, bunch locations…

The 2 parameters useful for tracking (_bunch_current and _bunch_spos, if I understand well, and possibly more) can be easily computed when they are needed, that is when interfacing with atpass. Possibly using the properties mentioned above. This precisely what lattice_pass and patpass are meant for. The interface to atpass is considered as private (apart form the basic use of tracking through a list, not a Lattice). So you are still free to work on lattice_pass and atpass, while the Lattice object will be stable.

I think we are now down to a question of taste... I prefer it this way I think it is more readable / understandable. With what you propose I have to modify the interface of 2 functions. Anyway I will do it because I need this branch merged... I hope after that it will be ok.

[swhite2401]

Ok I have modified the implementation as suggested

[lfarv]

I prefer it this way I think it is more readable / understandable

I can't believe that… The new version is so simpler !

Anyway, with the comments above, I think it can be merged. There are a few details that can be solved later:

• Due to the nbunch appearing in the _disp_attributes, saving and loading a lattice in the .repr format gives an unexpected result: the re-created lattice has a "normal" nbunch meaningless attribute, but a default filling pattern. Not crucial.
• The periodicity is not properly taken into account. Not critical, because a periodic lattice is not really adapted for multi-bunch tracking, unless the bunch pattern follows the same periodicity, which is a limitation. I guess you don't want to touch that, I'll take care later.

I'm a bit worried that "fillpattern' is not exactly the read/write property associated with _fillpattern: it would be more intuitive. Not critical, can be changed later.
I would also add a "bunch list" property, as another way to visualise the filling pattern.

So feel free to merge when you like.

[lfarv]

After minor modifications

[swhite2401]

I prefer it this way I think it is more readable / understandable

I can't believe that… The new version is so simpler !

Anyway, with the comments above, I think it can be merged. There are a few details that can be solved later:

* Due to the `nbunch` appearing in the `_disp_attributes`, saving and loading a lattice in the .repr format gives an unexpected result: the re-created lattice has a "normal" `nbunch` meaningless attribute, but a default filling pattern. Not crucial.

* The periodicity is not properly taken into account. Not critical, because a periodic lattice is not really adapted for multi-bunch tracking, unless the bunch pattern follows the same periodicity, which is a limitation. I guess you don't want to touch that, I'll take care later.

I'm a bit worried that "fillpattern' is not exactly the read/write property associated with _fillpattern: it would be more intuitive. Not critical, can be changed later. I would also add a "bunch list" property, as another way to visualise the filling pattern.

So feel free to merge when you like.

Ok final review I hope, I have implemated all your comments I believe.

Concerning the saving and loading of .repr format, I now force everything to default, at least value are consistent even though you lose the information on the fillpattern.

Concerning you comment on periodicity, I disagree. It is perfectly fine to simulate a non periodic fill pattern on a periodic lattice, both are independent, periodicity is already handled through the harmonic number. Please do not change this, adding more restrictions will be counter-productive.
However, it is also true that it does not make any sense to do multi-particle tracking with periodicity !=1 so I think this is a non issue.

I did more changes that expected, to I request for a last review if you have time...

[swhite2401]

By the way, I very often have to re-run the matlab test several times for them to complete properly, is this a known issue?

[lfarv]

I also hope it's the final one!

[lfarv]

By the way, I very often have to re-run the matlab test several times for them to complete properly, is this a known issue?

For a few days, I also noticed strange cancellations of Matlab test runs. Even before the test itself starts. No idea why…