Skip to content
Snippets Groups Projects

Intrabeam scattering tracking implementation

Merged GUBAIDULIN requested to merge Ibs-implementation into develop

Hello Salah,

Nicely done! Let's use this interface to review your code. One major thing is that documentation is missing, but we can fix this later when you solve all the other requests. Here are my requests on your IBS implementation right now.

  1. (lines 13-15) You could move these imports inside your IBS class because they are only used by the IBS part of the code.
  2. (line 535) The number of partitions cannot be selected without modifying the source code.
  3. (line 536) I believe your self.dt is the same as self.ring.T0.
  4. (line 538) You can import the classical electron radius from scipy.constants.physical_constants. However, this should be more general. Consider a situation when the code is used to compute the IBS of a hadron/proton accelerator. In that case, would you still use classical electron radius in the IBS formulas?
  5. (line 541) There's no way to select the IBS model to compute T_ibs without modifying the source code.
  6. (line 551,552) Why not use the same assignment as in line 546 (beta functions).
  7. (lines 563-584) All three functions are the same. The only difference is their arguments. It should be a single function f (a, b, q), and you call it in your code f (a, b, q), f (1/a, b/a, q/a), and f (1/b, b/a, q/b). This will reduce the number of lines in the code and make it more readable.
  8. (line 604-612) This is tracking. It should be inside the track() function. Functions get_T_ibs() should compute the T_ibs.
  9. (line 621, 622) Same as (line 551, 552)
  10. (line 666-676) Same consideration as (line 563-584)
  11. (line 545, 614, 716, 770) These two functions share some similarities. You probably should create a separate function that does this shared part (like averaging different positions 's'). Then you call get_T_CIMP() or get_T_Piwinski() from it.
  12. (line 797-815) Same consideration as (line 563-584, line 666-676)
  13. (line 854) All the tracking related functionality (manipulation of x, xp, y, yp, delta, tau) and related 'kicks' should be implemented here. Model selection should be done when the object is initialised.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • GUBAIDULIN
  • GUBAIDULIN
    • Resolved by GUBAIDULIN

      Hi Salah, Looks good. There are a few things left to do here besides my specific threads above.

      1. There is some common functionality between all your functions for get_T_model> (). For example, in many (most) models, A is computed; in all models, you compute T_ibs at locations s and then average them. In principle, all common functionality like that should be written only once.
      2. Some things like H-functions are constant (depend only on Twiss alpha/beta/gamma) -> can be computed in init()
      3. You need to generate documentation for the class and all functions in numpy docs format. Also, by convention in mbtrack2 we add references in the documentation. So, you'd need to add references to all models that you implemented to papers where the model is described. (and the same for Zampetakis kick).
  • GUBAIDULIN added 4 commits

    added 4 commits

    • e4bbee58...3a5cabb6 - 3 commits from branch develop
    • 87f607a9 - Accepted incoming changes from branch 'develop' into 'Ibs-implementation'

    Compare with previous version

  • FEDDAOUI added 2 commits

    added 2 commits

    • 2dfa0e5a - improved readability (deleted redundant loop)
    • 3ae1a1a0 - added particle type and optics partition number

    Compare with previous version

  • FEDDAOUI added 1 commit

    added 1 commit

    Compare with previous version

  • GUBAIDULIN
  • GUBAIDULIN
  • GUBAIDULIN
  • GUBAIDULIN
  • GUBAIDULIN
  • GUBAIDULIN
  • GUBAIDULIN
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading