Skip to content
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

Fix bug in at.c #321

Merged
merged 4 commits into from
Oct 22, 2021
Merged

Fix bug in at.c #321

merged 4 commits into from
Oct 22, 2021

Conversation

lfarv
Copy link
Contributor

@lfarv lfarv commented Oct 20, 2021

elempass was crashing when called several times with a python integrator. This is fixed

Few other fixes related to at.c

@lfarv lfarv added Python For python AT code bug fix labels Oct 20, 2021
@lfarv lfarv requested a review from swhite2401 October 20, 2021 18:59
@swhite2401
Copy link
Contributor

You are using python passmethods??

Very nice! Much simpler in fact! I have tested it using the example pyDriftPass.py, everything looks fine.

There is one thing that was pointed out to me though, the examples pyDrfitPass.py and PyIdentityPass.py are returning r_in in at/integrators when r_in is modified in-place so the return statement is useless.
I think that is correct, if you agree could we correct these in this PR?

Then I have a question: I personally never use elementpass but rather atpass([element]) is there any advantage to it?

@lfarv
Copy link
Contributor Author

lfarv commented Oct 21, 2021

I agree that the return value of python integrators is ignored. So we can return None. Returning the updated r_in may be interesting if you want to call the integrator directly, as in Matlab (but which is not possible in python for C integrators). So should we remove this ?

Concerning the use of these functions:

  1. as it was discussed some time ago, I consider the atpass and elempass C functions as private and favour the use of their "public" python interfaces lattice_pass and element_pass which are safer. I still intend to rename them with an underscore (with a stub tor backward compatibility).
  2. element_pass avoids a significant overhead in lattice_pass and directly calls the integrator. So it is more efficient (no idea of the difference), while giving the same result.

By the way, it is while trying to modify the test functions to use the lattice_pass and element_pass that I discovered the bug. Concerning the use of python integrators, I used one for prototyping the "ChangeReferencePass" for tapering with dipoles (still pending), and for that purpose it is very convenient.

@lfarv
Copy link
Contributor Author

lfarv commented Oct 21, 2021

Few other fixes related to at.c:

  • another memory leak,
  • the openMP status provided by the C function isopenmp is publicly available in at.DConstant.openmp

@lfarv lfarv merged commit f79696a into master Oct 22, 2021
@lfarv lfarv deleted the fix_at_c branch October 22, 2021 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Python For python AT code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants