Conversation
|
The CI failure is caused by a bug in pytac 0.6.2 which is being worked on |
T-Nicholls
left a comment
T-Nicholls
left a comment
There was a problem hiding this comment.
Looks fine to me, but obviously I need to see the corresponding changes to Pytac & Virtac
ptsOSL
force-pushed
the
diamond2-conversion
branch
3 times, most recently
from
ccfaa47 to
1e27cd9
Compare
MJGaughran
left a comment
MJGaughran
left a comment
There was a problem hiding this comment.
The logic related to RING and THERING is confusing, and I'm not convinced we're unable to at least tidy it up a little bit.
Otherwise, you will want to get all tests to pass on this branch. I imagine the pytac PR will resolve this.
| % Correct dimension order if necessary. | ||
| if size(RING, 1) == 1 | ||
| RING = permute(RING, [2 1]); | ||
| if size(THERING, 1) == 1 |
There was a problem hiding this comment.
I would recommend using a separate variable name here and in the rest of the file. If it is using RING from MML, it is afterwards referred to as THERING, which is confusing.
There was a problem hiding this comment.
E.g. something like mml_ring
There was a problem hiding this comment.
It also makes the comments confusing, since 'THERING' can sometimes refer to the MML THERING object, and at other times to our use of the variable in this script.
There was a problem hiding this comment.
I have made some changes to this. We now use ATIP_RING. If a file containing an ATIP_RING is supplied, we use that, if not then we initialise ATIP_RING from THERING (if it exists). We then modify ATIP_RING and dont change THERING, finally we save ATIP_RING to either the file supplied or a new file called lattice.mat
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #54 +/- ##
=======================================
Coverage 86.73% 86.73%
=======================================
Files 6 6
Lines 392 392
=======================================
Hits 340 340
Misses 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This conversion script should work for both diamond I and diamond II ringmodes. 48.mat is the latest DII ringmode.
Added a single place to define the ringmodes to tests. Created a combined fixture for loading both a pytac and atip lattice. Added a new test which tests just loading of the atip lattice.
Rick added these
Also tidy up handling of missing RING and THERING
This is not specified in any modern lattice files, so we do not need to check for this anymore. Checked for 48, I04, DIAD, VMX
Sanity check that a HSTR/VSTR after a sextupole has, length 0, which should confirm it is part of the sextupole.
ptsOSL
force-pushed
the
diamond2-conversion
branch
from
240e622 to
6388498
Compare
I also had to regenerate the three mat files so that they store the variable ATIP_RING instead of RING
ptsOSL
force-pushed
the
diamond2-conversion
branch
from
6388498 to
03c2619
Compare
3 participants
Changes to make the new DII lattice, 48.mat, work in the Virtac.
Relatively few changes were required.