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

BEAM sanity checks #689

Merged
merged 6 commits into from Oct 31, 2018
Merged

Conversation

kyrsjo
Copy link
Contributor

@kyrsjo kyrsjo commented Oct 24, 2018

Catch some problematic beam statements and emit warnings/fatal_error as appropriate and consistent with the rest of the file. Typos happen, and they are not always obvious; it is nice when the program detect a problem when it occurs and not later.

I think I kept to the style of the file, even fixed a formatting mistake another place in the file.

…as appropriate and consistent with the rest of the file.
@madcern
Copy link
Contributor

madcern commented Oct 25, 2018

I think this is a good idea to catch error in the input. We would however, need to update many tests (I should have a script that does it). I will just check with Laurent before I do the update.

@kyrsjo
Copy link
Contributor Author

kyrsjo commented Oct 26, 2018

Great, thanks! I don't know how to see the actual output of the individual tests, is it that it is actually catching problematic input files, spitting out warnings? Or is it even catching pc < 0?

@tpersson
Copy link
Contributor

Some of them are simply the warning which then changes the output put some are more serious. To see the results from the test you have to make numdiff and then simply make tests-all (or have a look at the Travis build.

@kyrsjo
Copy link
Contributor Author

kyrsjo commented Oct 30, 2018

Hi, thanks! By gleaming at the Travis output I figured out how to run the test suite. It seems that the issue is that I get an overspecification warning from the beam command at the very beginning of the output, and this throws off the rest of the test:

$ ../../madx-linux64-gnu test-table.madx 
++++++ warning: Both energy and pc specified; pc was ignored.
++++++ warning: Both energy and gamma specified; gamma was ignored.
++++++ warning: Both energy and beta specified; beta was ignored.
++++++ warning: Both energy and bhro specified; brho was ignored.

  ++++++++++++++++++++++++++++++++++++++++++++
  +     MAD-X 5.04.02  (64 bit, Linux)       +
  + Support: mad@cern.ch, http://cern.ch/mad +
  + Release   date: 2018.10.03               +
  + Execution date: 2018.10.30 15:21:49      +
  ++++++++++++++++++++++++++++++++++++++++++++
Option, -info; // avoid redefinition info

This bogus warning of course has to be avoided... Time for some digging!

@coveralls
Copy link

coveralls commented Oct 31, 2018

Coverage Status

Coverage increased (+0.02%) to 53.836% when pulling b67a7d6 on kyrsjo:beam-warnings into 97e8d8c on MethodicalAcceleratorDesign:master.

@kyrsjo
Copy link
Contributor Author

kyrsjo commented Oct 31, 2018

OK, that fixed all the bugs - some in the code (initialization) and some in the test test-sequence-3.

What remains seems to just be coveralls - but I can add a new test to purposely trigger the warnings?

Copy link
Contributor

@tpersson tpersson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent! Also very nice with a test!

@tpersson tpersson merged commit 9210d1a into MethodicalAcceleratorDesign:master Oct 31, 2018
@kyrsjo kyrsjo deleted the beam-warnings branch October 31, 2018 10:25
@kyrsjo
Copy link
Contributor Author

kyrsjo commented Oct 31, 2018

Yeah, I figured out that there was already a relevant test so I just added it to that.

Thanks for merging!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants