Prettify by alongd · Pull Request #1744 · ReactionMechanismGenerator/RMG-Py

alongd · 2019-10-03T02:34:03Z

Motivation or Problem

As noted in #1726 by @yunsiechung (Thanks!), Arkane's prettify functionality got broken in the Py3 transition.

Description of Changes

Instead of adapting Abstract Syntax Trees to the Py3 changes (which I don't know how to do right now), this PR suggests to use a simple combination of pprint.pformat and RMGObject.as_dict(). This might be advantageous since theoretically the object can be rebuilt using from_dict().

Testing

A test was added, showing how this feature performs for a complex object such as Conformer.

Point for discussion

Arkane's thermo and kinetics blocks we output are not objects, and have no real pythonic form. Here I added a tentative fix for them, we might want to reconsider how to output them better.
I'd like to try removing (not fixing) thermo and kinetic output for output.py, and instead (and finally) save respective RMG library files. We will still output the data in the table form for both in output.py.

Here's an example output:

# Conformer for C7H7:
{   'E0': {   'class': 'ScalarQuantity',
              'units': 'kJ/mol',
              'value': 193.74920136178187},
    'class': 'Conformer',
    'coordinates': {   'class': 'ArrayQuantity',
                       'units': 'angstroms',
                       'value': {   'class': 'np_array',
                                    'object': [   [0.0, 0.0, -1.90931],
                                                  [0.0, 1.20973, -1.20462],
                                                  [   0.0,
                                                      1.2163699999999997,
                                                      0.17817],
                                                  [0.0, 0.0, 0.92089],
                                                  [   0.0,
                                                      -1.2163699999999997,
                                                      0.17817],
                                                  [0.0, -1.20973, -1.20462],
                                                  [0.0, 0.0, -2.99302],
                                                  [0.0, 2.14897, -1.74692],
                                                  [0.0, 2.15757, 0.71765],
                                                  [0.0, -2.15757, 0.71765],
                                                  [0.0, -2.14897, -1.74692],
                                                  [0.0, 0.0, 2.32493],
                                                  [0.0, 0.92727, 2.88399],
                                                  [0.0, -0.92727, 2.88399]]}},
    'mass': {   'class': 'ArrayQuantity',
                'units': 'amu',
                'value': {   'class': 'np_array',
                             'object': [   12.000000000000002,
                                           12.000000000000002,
                                           12.000000000000002,
                                           12.000000000000002,
                                           12.000000000000002,
                                           12.000000000000002,
                                           1.00782503224,
                                           1.00782503224,
                                           1.00782503224,
                                           1.00782503224,
                                           1.00782503224,
                                           12.000000000000002,
                                           1.00782503224,
                                           1.00782503224]}},
    'modes': [   {   'class': 'IdealGasTranslation',
                     'mass': {   'class': 'ScalarQuantity',
                                 'units': 'amu',
                                 'value': 91.05478},
                     'quantum': False},
                 {   'class': 'NonlinearRotor',
                     'inertia': {   'class': 'ArrayQuantity',
                                    'units': 'amu*angstrom^2',
                                    'value': {   'class': 'np_array',
                                                 'object': [   91.05665505889641,
                                                               186.6754595575174,
                                                               277.7326559036768]}},
                     'quantum': False,
                     'rotationalConstant': {   'class': 'ArrayQuantity',
                                               'units': 'cm^-1',
                                               'value': {   'class': 'np_array',
                                                            'object': [   0.18513340999012978,
                                                                          0.09030447329984698,
                                                                          0.060697324189327787]}},
                     'symmetry': 2},
                 {   'class': 'HarmonicOscillator',
                     'frequencies': {   'class': 'ArrayQuantity',
                                        'units': 'cm^-1',
                                        'value': {   'class': 'np_array',
                                                     'object': [   199.38075265757388,
                                                                   360.5363271321874,
                                                                   413.7949151525557,
                                                                   480.34741903286914,
                                                                   536.2845084113071,
                                                                   630.7231437836404,
                                                                   687.1180869560582,
                                                                   709.613210214998,
                                                                   776.6617864901739,
                                                                   830.4036310625469,
                                                                   834.386032685694,
                                                                   901.8407934891261,
                                                                   973.4982807999646,
                                                                   975.1481503393776,
                                                                   993.3485157754649,
                                                                   998.6064486942171,
                                                                   1040.1386185625333,
                                                                   1120.6925785487256,
                                                                   1179.2234881852514,
                                                                   1189.0708173043222,
                                                                   1292.8647176431568,
                                                                   1332.909949640503,
                                                                   1357.1753414643513,
                                                                   1479.4620076104507,
                                                                   1495.3606146408765,
                                                                   1507.9144088991889,
                                                                   1583.1430987057158,
                                                                   1604.628263865009,
                                                                   3156.8525679482696,
                                                                   3170.2206892536133,
                                                                   3172.7811150780485,
                                                                   3185.0534994349905,
                                                                   3189.799070842567,
                                                                   3203.232858579444,
                                                                   3253.9889510503353]}},
                     'quantum': True},
                 {   'class': 'HinderedRotor',
                     'fourier': {   'class': 'ArrayQuantity',
                                    'units': 'kJ/mol',
                                    'value': {   'class': 'np_array',
                                                 'object': [   [   -0.31592326520854164,
                                                                   -27.766519216358134,
                                                                   0.177678139795233,
                                                                   3.2437014555229857,
                                                                   0.050951495010736314],
                                                               [   -0.0016495340958863763,
                                                                   -0.0021925030855088038,
                                                                   -0.003863963373036643,
                                                                   -0.0009120676025288776,
                                                                   0.0027420643405067606]]}},
                     'frequency': 0.0,
                     'inertia': {   'class': 'ScalarQuantity',
                                    'units': 'amu*angstrom^2',
                                    'value': 1.7001286526557986},
                     'quantum': True,
                     'rotationalConstant': {   'class': 'ScalarQuantity',
                                               'units': 'cm^-1',
                                               'value': 9.915501998636937},
                     'semiclassical': False,
                     'symmetry': 2}],
    'number': {   'class': 'ArrayQuantity',
                  'value': {   'class': 'np_array',
                               'object': [   6.0,
                                             6.0,
                                             6.0,
                                             6.0,
                                             6.0,
                                             6.0,
                                             1.0,
                                             1.0,
                                             1.0,
                                             1.0,
                                             1.0,
                                             6.0,
                                             1.0,
                                             1.0]}},
    'optical_isomers': 1,
    'spin_multiplicity': 2}

codecov · 2019-10-03T04:04:10Z

Codecov Report

Merging #1744 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #1744   +/-   ##
======================================
  Coverage    32.6%   32.6%           
======================================
  Files          87      87           
  Lines       26115   26115           
  Branches     6875    6875           
======================================
  Hits         8516    8516           
+ Misses      16641   16629   -12     
- Partials      958     970   +12

Impacted Files	Coverage Δ
rmgpy/data/statmech.py	`42.2% <0%> (ø)`	⬆️
rmgpy/rmg/pdep.py	`12.21% <0%> (ø)`	⬆️
rmgpy/reaction.py	`0% <0%> (ø)`	⬆️
rmgpy/data/kinetics/family.py	`48.28% <0%> (ø)`	⬆️
rmgpy/yml.py	`15.71% <0%> (ø)`	⬆️
rmgpy/data/kinetics/database.py	`50.61% <0%> (ø)`	⬆️
rmgpy/molecule/molecule.py	`0% <0%> (ø)`	⬆️
rmgpy/statmech/ndTorsions.py	`59.78% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59c93db...3737291. Read the comment docs.

mliu49 · 2019-10-03T14:20:41Z

Prettify was broken because of changes to method names in PrettifyVisitor in 59d8060. It subclasses ast.NodeVisitor, which relies on having special methods named visit_classname, where classname may be Call, Str, Num, etc.

For now, I think we should revert the method renaming in this class to restore the original behavior. However, I do agree that we should reconsider our output formats in the near future.

alongd · 2019-10-03T18:44:10Z

OK, I reverted the PEP8 prettify changes, and added a test.
We'll find time to discuss a new format for Arkane's output.
Thanks for the guidance, @mliu49!

mliu49 · 2019-10-03T22:17:53Z

I added two more fixes and a unit test. Turns out that Python 3 handles negative numbers differently than Python 2...

alongd

Looking good! I added one comment/question

alongd · 2019-10-09T18:49:14Z

@mliu49, thanks for your excellent additions to this PR!
I rebased and squashed, should be ready to merge after the tests pass.

In Python 2, negative numbers were treated as a single node In Python 3, negative numbers are treated as a UnaryOp + a Num

mliu49 · 2019-10-09T22:44:06Z

Sorry, I merged another PR so I went ahead and rebased this again. I will merge after tests pass, unless you think we need a third reviewer.

alongd · 2019-10-10T10:30:25Z

@mliu49, I think this is ready to be merged. Thanks again

alongd requested a review from mjohnson541 October 3, 2019 02:34

alongd self-assigned this Oct 3, 2019

alongd added Arkane Py3 Status: Discussion Needed Discussion needed for further progress Type: User Experience labels Oct 3, 2019

alongd force-pushed the prettify branch from 0d298b7 to c4d7584 Compare October 3, 2019 17:54

alongd commented Oct 9, 2019

View reviewed changes

alongd force-pushed the prettify branch from 1645a2e to e043acb Compare October 9, 2019 18:47

alongd and others added 5 commits October 9, 2019 18:42

BugFix: Revert function renaming in Arkane output.py

2dc1fec

Tests: Added Arkane output test

1d669d3

Fix bug in visit_Call with duplicated identifier

3ccc0c2

Update handling of negative numbers in PrettifyVisitor

9292fb7

In Python 2, negative numbers were treated as a single node In Python 3, negative numbers are treated as a UnaryOp + a Num

Add another unit test for prettify function

3737291

mliu49 force-pushed the prettify branch from e043acb to 3737291 Compare October 9, 2019 22:43

alongd removed the Status: Discussion Needed Discussion needed for further progress label Oct 10, 2019

mliu49 approved these changes Oct 10, 2019

View reviewed changes

mliu49 merged commit 5d7c136 into master Oct 10, 2019

mliu49 deleted the prettify branch October 10, 2019 12:30

mliu49 mentioned this pull request Nov 26, 2019

RMG v3.0.0 Release Planning #1830

Closed

mliu49 mentioned this pull request Dec 16, 2019

RMG-Py v3.0.0 Release #1852

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prettify#1744

Prettify#1744
mliu49 merged 5 commits intomasterfrom
prettify

alongd commented Oct 3, 2019 •

edited

Loading

Uh oh!

codecov bot commented Oct 3, 2019 •

edited

Loading

Uh oh!

mliu49 commented Oct 3, 2019

Uh oh!

alongd commented Oct 3, 2019

Uh oh!

mliu49 commented Oct 3, 2019

Uh oh!

alongd left a comment

Uh oh!

alongd commented Oct 9, 2019

Uh oh!

mliu49 commented Oct 9, 2019

Uh oh!

alongd commented Oct 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

alongd commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation or Problem

Description of Changes

Testing

Point for discussion

Uh oh!

codecov bot commented Oct 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mliu49 commented Oct 3, 2019

Uh oh!

alongd commented Oct 3, 2019

Uh oh!

mliu49 commented Oct 3, 2019

Uh oh!

alongd left a comment

Choose a reason for hiding this comment

Uh oh!

alongd commented Oct 9, 2019

Uh oh!

mliu49 commented Oct 9, 2019

Uh oh!

alongd commented Oct 10, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alongd commented Oct 3, 2019 •

edited

Loading

codecov bot commented Oct 3, 2019 •

edited

Loading