star-bnl / star-sw

Core software for STAR experiment
26 stars 63 forks source link

Compile AgML under python 3 #646

Open klendathu2k opened 2 months ago

klendathu2k commented 2 months ago

This is a draft PR for compiling AgML using python 3.

There were only a few types of changes needed to get AgML to run under python 3. 1) Python 3 requires () in print and presumably assert statements. 2) Exceptions have been moved from the "exceptions" to the "builtins" module. 3) Python 3 needs the full path to the module, even if we are importing the module from a module in the same directory. 4) Python 3 requires consistent use of spaces vs tabs for indentation purposes (showed up in one file... I think when I went on a quest for a new editor... as if emacs and vi are not enough.) 5) Python3 has changed dictionaries. The iteritems() method is removed. The iter() method, which is common between python 2 and 3, should be used instead. (Slightly slower... and return list may now be bound differently between python2 and 3...) 5a) Minor issue w/ using "pop" to access dictionary keys resulting from above... get was a perfectly good substitute 6) And there is something in the legacy compat pams/geometry/tpcegeo/tpcegeo.g file that throws a unicode error. Ignoring the error seems to fix the problem w/ no observable difference in the output.

The refactoring above may or may not leave the geometry model invariant. It is sufficient to show that the output of the AgML codes is identical, up to whitespace differences and irrelevant re-orderings of declaration statements.

On the c++ side, we are good. The codes produced using python2 is equivalent to the python3 code. Some differences are observed in the order in which (for instance) shape parameters are output. This is a consequence of the change from iteritems() to iter()... and if I took the python2 output from after the refactoring, I would probably not even see this difference.

On the Mortran side, we have additional differences. The rules for breaking lines at column 72 seem to be broken. Will need to solve this problem before converting this from draft.

klendathu2k commented 2 months ago

Just a quick note. The geometry model is invariant IF the C++ and MORtran source codes generated by AgML are functionally identical when we use python 2.7 versus python 3.

As noted above, we have this functional equivalence in the C++ code. On the MORtran side, we needed to fix the code which determines line breaks when printing out a MORtran array. Python2 and python3 handle integer division differently. (The former returns an int always, the latter a floating point value).

Once fixed, the MORtran generated using python 2.7 and 3.9 is equivalent up to (1) differences in whitespace, (2) different ordering of variable assignments, and (3) different ordering of AgSTAR keyword arguments. These differences have no observable impact on the final geometry. These are all consequences of switching from the iteritems() method to the iter() method for iterating on a dictionary.

As Dmitri noted, this will be ready to merge once python 3 is made default. Should discuss at next S&C meeting.

veprbl commented 2 weeks ago

Integer division in python3 is done using the // operator.