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

Compile AgML under python 3 #646

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

klendathu2k
Copy link
Contributor

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.

Copy link
Member

@plexoos plexoos left a comment

Choose a reason for hiding this comment

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

Looks good!

To really test it we need to load python3 module by default

mgr/Dyson/Export/Mortran.py Outdated Show resolved Hide resolved
mgr/Dyson/Export/Mortran.py Outdated Show resolved Hide resolved
mgr/Dyson/Export/Mortran.py Outdated Show resolved Hide resolved
mgr/Dyson/Export/AgMLExceptions.py Outdated Show resolved Hide resolved
klendathu2k and others added 4 commits February 5, 2024 21:49
Co-authored-by: Dmitri Smirnov <[email protected]>
Co-authored-by: Dmitri Smirnov <[email protected]>
... which had been used for a test

Co-authored-by: Dmitri Smirnov <[email protected]>
…hon 3) if ImportError raised

Co-authored-by: Dmitri Smirnov <[email protected]>
@klendathu2k
Copy link
Contributor Author

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
Copy link
Member

veprbl commented Apr 16, 2024

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

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.

3 participants