Fix bugs in Chemkin thermo entry element reading/writing #1636

mliu49 · 2019-06-19T22:26:38Z

Motivation or Problem

RMG Chemkin files break Cantera ck2cti for species with 5 or more elements. #1634

Description of Changes

For species with 5 elements, prefer putting the 5th element in columns 74-78 of the first line
For species with more than 5 elements, fix the line continuation syntax by switching to space delimited entries instead of continuing the {0!s:<2}{1:>3d} approach.
Add read support for thermo entries using the line continuation syntax
Add unit tests for reading and writing thermo entries

Testing

Should test with ck2cti and perhaps Chemkin as well. I have not done so yet.

codecov · 2019-06-19T23:13:11Z

Codecov Report

❗ No coverage uploaded for pull request base (master@7e62adf). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1636   +/-   ##
=========================================
  Coverage          ?   41.85%           
=========================================
  Files             ?       82           
  Lines             ?    21574           
  Branches          ?     5653           
=========================================
  Hits              ?     9030           
  Misses            ?    11553           
  Partials          ?      991

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 7e62adf...44d7554. Read the comment docs.

rwest · 2019-06-20T15:44:54Z

Ugh. This then led me to dig further and ask is the columns 74-78 extension official and/or in Chemkin or Cantera?

It seems that it is official, and in Chemkin, but it is not (yet) read by Cantera, although Cantera DOES read the extended line syntax (now that we have it correctly white-space separated)

This is part of Cantera's ck2cti

    def readThermoEntry(self, lines, TintDefault):
        """
        Read a thermodynamics entry for one species in a Chemkin-format file
        (consisting of two 7-coefficient NASA polynomials). Returns the label of
        the species, the thermodynamics model as a :class:`MultiNASA` object, the
        elemental composition of the species, and the comment/note associated with
        the thermo entry.
        """
        identifier = lines[0][0:24].split()
        species = identifier[0].strip()

        if len(identifier) > 1:
            note = ''.join(identifier[1:]).strip()
        else:
            note = ''

        # Normal method for specifying the elemental composition
        composition = self.parseComposition(lines[0][24:44], 4, 5)

        # Chemkin-style extended elemental composition: additional lines
        # indicated by '&' continuation character on preceding lines. Element
        # names and abundances are separated by whitespace (not fixed width)
        if lines[0].rstrip().endswith('&'):
            complines = []
            for i in range(len(lines)-1):
                if lines[i].rstrip().endswith('&'):
                    complines.append(lines[i+1])
                else:
                    break
            lines = [lines[0]] + lines[i+1:]
            comp = ' '.join(line.rstrip('&\n') for line in complines).split()
            composition = {}
            for i in range(0, len(comp), 2):
                composition[comp[i].capitalize()] = int(comp[i+1])

        # Non-standard extended elemental composition data may be located beyond
        # column 80 on the first line of the thermo entry
        if len(lines[0]) > 80:
            elements = lines[0][80:]
            composition2 = self.parseComposition(elements, len(elements)//10, 10)
            composition.update(composition2)

        if not composition:
            raise InputParseError("Error parsing elemental composition for "
                                  "species '{0}'".format(species))

The columns 74-78 syntax is in the ckinterp.f source code comments

C     OPTIONAL THERMODYNAMIC DATA: (Subroutine CKTHRM)
C     (If this feature is not used, thermodynamic properties are
C     obtained from a CHEMKIN database.)  The format for this option
C     is the word 'THERMO' followed by any number of 4-line data sets:
C
C     Line 1: species name, optional comments, elemental composition,
C             phase, T(low), T(high), T(mid), additional elemental
C             composition, card number (col. 80);
C             format(A10,A14,4(A2,I3),A1,E10.0,E10.0,E8.0,(A2,I3),I1)

and the source code

      ICOL = 20
      DO 60 I = 1, 5
         ICOL = ICOL + 5
         IF (I .EQ. 5) ICOL = 74
         ELEM  = LINE(1)(ICOL:ICOL+1)

and it's also documented in the 1980 Sandia report of the original CHEMKIN
https://prod-ng.sandia.gov/techlib-noauth/access-control.cgi/1980/808003.pdf (p60)

...so we should open an issue (or pull request) to support it in Cantera.

But until that's merged and released, doing this will make Chemkin files that Cantera can't read.
Should we just do the extended line syntax?

I think I'd rather inconvenience Chemkin-II users than Cantera 2.4.0 users. I know several of the latter ;-)

Of course, we could also include our own slightly modified ck2cti in RMG. It's a stand-alone Python script that doesn't need all of Cantera.

rwest · 2019-06-20T21:16:52Z

For reference: Cantera/cantera#656

rwest · 2019-11-13T20:30:55Z

I rebased @mliu49's PR onto the new master, converting them to Python 3 and RMG 3 in the process, and added another couple of commits. Hope you don't mind that I force pushed.
I think this is now ready for a review.

mliu49 · 2019-11-22T20:08:25Z

I did some additional refactoring of the thermo writing method and squashed the commits together.

rwest

Haven't actually run it or the tests, but looking at the code on github I like the way you have refactored it.

rwest · 2019-11-23T03:57:40Z

rmgpy/chemkinTest.py

+"""
+
+        self.entry2 = """CH3NO2X                                     G   300.000  3000.000  650.73      1&
+C 1 H 3 N 1 O 2 X 1


Probably these have to be 5-column wide blocks like C 1 H 3 N 1 O 2 X 1 now?
Haven't actually run the code or the unit tests to check.

The 5-column wide blocks refers to the original method of placing them in the first line. The extended line is still space delimited. The tests currently pass.

Ah yes, I misread something. Looks good. Can I "approve"

rwest · 2019-11-23T03:58:12Z

rmgpy/chemkinTest.py

+"""
+
+        self.entry3 = """CH3NO2SX                                    G   300.000  3000.000  650.73      1&
+C 1 H 3 N 1 O 2 S 1 X 1


Same for these, I imagine

Using the same algorithm as Molecule.get_formula() method which says iff there is any Carbon then C and H come first, and everything else is alphabetic. Before this, it seemed to be unstable, at least with respect to small changes in RMG, even if it were stable from one run to another.

For species with 4 elements or less, put them in the first line For species with 5 elements or more, put them on the next line using the space delimited syntax. Although the CHEMKIN manual specifies that one may put the 5th element atom count in columns 74-78 of row 1, in practice nobody does this, and Cantera cannot read it if we do, so this commit stops us from doing this. Instead if there are 5 or more elements present, we use the extended syntax (that we were using for 6+ elements). For more details see discussion at Cantera/cantera#656

Add unit tests for various element count syntaxes Fix incorrect index for element count in columns 74-78 (should start at column 73 using zero-indexing)

mliu49 added Topic: Chemkin Type: Bug labels Jun 19, 2019

mliu49 requested review from rwest and goldmanm June 19, 2019 22:26

mliu49 self-assigned this Jun 19, 2019

mliu49 added the Status: Discussion Needed Discussion needed for further progress label Jun 20, 2019

amarkpayne added the After Py3 To be addressed after Python 3 transition label Jul 11, 2019

rwest force-pushed the ck_thermo branch from 7f9aafd to 529a159 Compare November 13, 2019 20:29

rwest force-pushed the ck_thermo branch 2 times, most recently from 60d9467 to a99fcdc Compare November 14, 2019 01:28

mliu49 added For Next Release and removed After Py3 To be addressed after Python 3 transition labels Nov 19, 2019

mliu49 force-pushed the ck_thermo branch from a99fcdc to 3470e51 Compare November 22, 2019 20:04

mliu49 requested review from amarkpayne and removed request for goldmanm November 22, 2019 20:08

rwest reviewed Nov 23, 2019

View reviewed changes

rwest previously approved these changes Dec 1, 2019

View reviewed changes

rwest and others added 3 commits December 2, 2019 11:19

Add parsing for extended element counts in Chemkin thermo entries

44d7554

Add unit tests for various element count syntaxes Fix incorrect index for element count in columns 74-78 (should start at column 73 using zero-indexing)

mliu49 dismissed rwest’s stale review via 44d7554 December 2, 2019 16:19

mliu49 force-pushed the ck_thermo branch from 3470e51 to 44d7554 Compare December 2, 2019 16:19

amarkpayne approved these changes Dec 2, 2019

View reviewed changes

mliu49 merged commit a87d79f into master Dec 2, 2019

mliu49 deleted the ck_thermo branch December 2, 2019 19:02

This was referenced Dec 4, 2019

RMG v3.0.0 Release Planning #1830

Closed

Invalid chemkin thermo block generated for an adsorbate with 5 elements #1634

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

Fix bugs in Chemkin thermo entry element reading/writing #1636

Fix bugs in Chemkin thermo entry element reading/writing #1636

mliu49 commented Jun 19, 2019

codecov bot commented Jun 19, 2019 •

edited

Loading

rwest commented Jun 20, 2019

rwest commented Jun 20, 2019

rwest commented Nov 13, 2019

mliu49 commented Nov 22, 2019

rwest left a comment

rwest Nov 23, 2019

mliu49 Nov 25, 2019

rwest Dec 1, 2019

rwest Nov 23, 2019

Fix bugs in Chemkin thermo entry element reading/writing #1636

Fix bugs in Chemkin thermo entry element reading/writing #1636

Conversation

mliu49 commented Jun 19, 2019

Motivation or Problem

Description of Changes

Testing

codecov bot commented Jun 19, 2019 • edited Loading

Codecov Report

rwest commented Jun 20, 2019

rwest commented Jun 20, 2019

rwest commented Nov 13, 2019

mliu49 commented Nov 22, 2019

rwest left a comment

Choose a reason for hiding this comment

rwest Nov 23, 2019

Choose a reason for hiding this comment

mliu49 Nov 25, 2019

Choose a reason for hiding this comment

rwest Dec 1, 2019

Choose a reason for hiding this comment

rwest Nov 23, 2019

Choose a reason for hiding this comment

codecov bot commented Jun 19, 2019 •

edited

Loading