From b6de3d567b4900a5cbb27c0c8971443baac9adb2 Mon Sep 17 00:00:00 2001 From: Mark Goldman Date: Mon, 20 May 2019 21:14:14 -0400 Subject: [PATCH 1/4] Improve merge_model robustness Previously merge_models would error if no species or reactions found. This commit prevents the divide by zero error from stopping merge_models. --- rmgpy/tools/merge_models.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/rmgpy/tools/merge_models.py b/rmgpy/tools/merge_models.py index 6004e7ac85..693fff6220 100644 --- a/rmgpy/tools/merge_models.py +++ b/rmgpy/tools/merge_models.py @@ -122,10 +122,21 @@ def execute(inputModelFiles, **kwargs): finalModel = finalModel.merge(model) Nspec = len(finalModel.species) Nrxn = len(finalModel.reactions) - print 'Added {1:d} out of {2:d} ({3:.1f}%) unique species from model #{0:d}.'.format(i+1, Nspec - Nspec0, len(model.species), (Nspec - Nspec0) * 100. / len(model.species)) - print 'Added {1:d} out of {2:d} ({3:.1f}%) unique reactions from model #{0:d}.'.format(i+1, Nrxn - Nrxn0, len(model.reactions), (Nrxn - Nrxn0) * 100. / len(model.reactions)) - - print 'The merged model has {0:d} species and {1:d} reactions'.format(len(finalModel.species), len(finalModel.reactions)) + if len(model.species) > 0: + print('Added {1:d} out of {2:d} ({3:.1f}%) unique species from model ' + '#{0:d}.'.format(i+1, Nspec - Nspec0, len(model.species), (Nspec - Nspec0) * 100. / len(model.species))) + else: + print('Added {1:d} out of {2:d} unique species from model ' + '#{0:d}.'.format(i+1, Nspec - Nspec0, len(model.species))) + + if len(model.reactions) > 0: + print('Added {1:d} out of {2:d} ({3:.1f}%) unique reactions from model ' + '#{0:d}.'.format(i+1, Nrxn - Nrxn0, len(model.reactions), (Nrxn - Nrxn0) * 100. / len(model.reactions))) + else: + print('Added {1:d} out of {2:d} unique reactions from model ' + '#{0:d}.'.format(i+1, Nrxn - Nrxn0, len(model.reactions))) + print('The merged model has {0:d} species and {1:d} reactions' + ''.format(len(finalModel.species), len(finalModel.reactions))) # Save the merged model to disk saveChemkinFile(outputChemkinFile, finalModel.species, finalModel.reactions) From 490fe750e772e1d6aa988378b7935e15cf3c5d17 Mon Sep 17 00:00:00 2001 From: MarkGoldman Date: Fri, 5 Jul 2019 11:42:39 -0400 Subject: [PATCH 2/4] Refactor merge_model This commit refactors merge_model to separate out the merging and saving of the model so that it can be effectively unittested. --- rmgpy/tools/merge_models.py | 56 ++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/rmgpy/tools/merge_models.py b/rmgpy/tools/merge_models.py index 693fff6220..c59790aa90 100644 --- a/rmgpy/tools/merge_models.py +++ b/rmgpy/tools/merge_models.py @@ -105,23 +105,46 @@ def execute(inputModelFiles, **kwargs): outputChemkinFile = os.path.join(wd, 'chem.inp') outputSpeciesDictionary = os.path.join(wd, 'species_dictionary.txt') outputTransportFile = os.path.join(wd, 'tran.dat') if transport else None - - # Load the models to merge + + models = get_models_to_merge(inputModelFiles) + + finalModel = combine_models(models) + + # Save the merged model to disk + saveChemkinFile(outputChemkinFile, finalModel.species, finalModel.reactions) + saveSpeciesDictionary(outputSpeciesDictionary, finalModel.species) + if transport: + saveTransportFile(outputTransportFile, finalModel.species) + + print 'Merged Chemkin file saved to {0}'.format(outputChemkinFile) + print 'Merged species dictionary saved to {0}'.format(outputSpeciesDictionary) + if transport: + print 'Merged transport file saved to {0}'.format(outputTransportFile) + +def get_models_to_merge(input_model_files): + """ + Reads input file paths and creates a list of ReactionModel + """ models = [] - for chemkin, speciesPath, transportPath in inputModelFiles: + for chemkin, speciesPath, transportPath in input_model_files: print 'Loading model #{0:d}...'.format(len(models)+1) model = ReactionModel() model.species, model.reactions = loadChemkinFile(chemkin, speciesPath, transportPath=transportPath) models.append(model) + return models - finalModel = ReactionModel() +def combine_models(models): + """ + Takes in a list of ReactionModels and and merges them into a single ReactionModel + """ + final_model = ReactionModel() for i, model in enumerate(models): print 'Ignoring common species and reactions from model #{0:d}...'.format(i+1) - Nspec0 = len(finalModel.species) - Nrxn0 = len(finalModel.reactions) - finalModel = finalModel.merge(model) - Nspec = len(finalModel.species) - Nrxn = len(finalModel.reactions) + Nspec0 = len(final_model.species) + Nrxn0 = len(final_model.reactions) + final_model = final_model.merge(model) + Nspec = len(final_model.species) + Nrxn = len(final_model.reactions) if len(model.species) > 0: print('Added {1:d} out of {2:d} ({3:.1f}%) unique species from model ' '#{0:d}.'.format(i+1, Nspec - Nspec0, len(model.species), (Nspec - Nspec0) * 100. / len(model.species))) @@ -136,16 +159,5 @@ def execute(inputModelFiles, **kwargs): print('Added {1:d} out of {2:d} unique reactions from model ' '#{0:d}.'.format(i+1, Nrxn - Nrxn0, len(model.reactions))) print('The merged model has {0:d} species and {1:d} reactions' - ''.format(len(finalModel.species), len(finalModel.reactions))) - - # Save the merged model to disk - saveChemkinFile(outputChemkinFile, finalModel.species, finalModel.reactions) - saveSpeciesDictionary(outputSpeciesDictionary, finalModel.species) - if transport: - saveTransportFile(outputTransportFile, finalModel.species) - - print 'Merged Chemkin file saved to {0}'.format(outputChemkinFile) - print 'Merged species dictionary saved to {0}'.format(outputSpeciesDictionary) - if transport: - print 'Merged transport file saved to {0}'.format(outputTransportFile) - + ''.format(len(final_model.species), len(final_model.reactions))) + return final_model From 0c5c468425713d2775d45dbdbab8e3100261c14e Mon Sep 17 00:00:00 2001 From: MarkGoldman Date: Fri, 5 Jul 2019 11:45:23 -0400 Subject: [PATCH 3/4] Add unittests for merge_model This commit adds unittests to test merge_model functionality. --- rmgpy/tools/data/diffmodels/chem3.inp | 58 +++++++++++++ .../data/diffmodels/species_dictionary3.txt | 42 ++++++++++ rmgpy/tools/merge_modelsTest.py | 84 +++++++++++++++++++ 3 files changed, 184 insertions(+) create mode 100644 rmgpy/tools/data/diffmodels/chem3.inp create mode 100644 rmgpy/tools/data/diffmodels/species_dictionary3.txt create mode 100644 rmgpy/tools/merge_modelsTest.py diff --git a/rmgpy/tools/data/diffmodels/chem3.inp b/rmgpy/tools/data/diffmodels/chem3.inp new file mode 100644 index 0000000000..12570fd6e4 --- /dev/null +++ b/rmgpy/tools/data/diffmodels/chem3.inp @@ -0,0 +1,58 @@ +ELEMENTS H C O N Ne Ar He Si S Cl END + +SPECIES + ethane + C + CH3 + C2H5 + CH2O(150) + H(6) +END + + + +THERM ALL + 300.000 1000.000 5000.000 +! GRI-Mech3.0 +ethane H 6C 2 G 100.000 5000.000 1002.57 1 + 2.56122991E+00 1.83185519E-02-7.41959133E-06 1.38553537E-09-9.75184837E-14 2 +-1.19064416E+04 7.14373636E+00 3.71890614E+00 7.87935091E-04 3.81270648E-05 3 +-4.17468853E-08 1.38611864E-11-1.14896639E+04 4.79187839E+00 4 + +C C 1 H 4 G100.000 5000.000 1084.14 1 + 9.08338537E-01 1.14539658E-02-4.57167062E-06 8.29175956E-10-5.66302025E-14 2 +-9.72000634E+03 1.39926820E+01 4.20540260E+00-5.35542811E-03 2.51118367E-05 3 +-2.13756810E-08 5.97499754E-12-1.01619428E+04-9.21233746E-01 4 + +CH3 C 1 H 3 G100.000 5000.000 1019.18 1 + 3.63559133E+00 4.67201977E-03-1.75880625E-06 3.19690509E-10-2.22178045E-14 2 + 1.61705114E+04 1.15112614E+00 3.91979381E+00 7.47713594E-04 8.15094523E-06 3 +-8.86667966E-09 2.89448590E-12 1.62584634E+04 4.90302919E-01 4 + +C2H5 C 2 H 5 G100.000 5000.000 900.30 1 + 5.15604319E+00 9.43151705E-03-1.81963219E-06 2.21237186E-10-1.43516109E-14 2 + 1.20641491E+04-2.91006043E+00 3.82189848E+00-3.43439154E-03 5.09287254E-05 3 +-6.20254195E-08 2.37093270E-11 1.30660102E+04 7.61621264E+00 4 + +CH2O(150) H 2C 1O 1 G 100.000 5000.000 1041.96 1 + 2.36095410E+00 7.66804276E-03-3.19770442E-06 6.04724833E-10-4.27517878E-14 2 +-1.42794809E+04 1.04457152E+01 4.13878818E+00-4.69514383E-03 2.25730249E-05 3 +-2.09849937E-08 6.36123283E-12-1.43493283E+04 3.23827482E+00 4 + +H(6) H 2 G100.000 5000.000 4448.37 1 + 2.50002506E+00-2.19247328E-08 7.18877672E-12-1.04676904E-15 5.71113259E-19 2 + 2.54741949E+04-4.45131850E-01 2.50000000E+00-9.83249483E-14 1.27659594E-16 3 +-5.39769234E-19 7.02032739E-22 2.54742178E+04-4.44972896E-01 4 + +END + + + +REACTIONS KCAL/MOLE MOLES + +CH3+CH3=ethane 8.260e+15 -1.400 1.000 + +ethane+CH3=C+C2H5 3.936e-02 4.340 8.000 + +END + diff --git a/rmgpy/tools/data/diffmodels/species_dictionary3.txt b/rmgpy/tools/data/diffmodels/species_dictionary3.txt new file mode 100644 index 0000000000..1c489c473b --- /dev/null +++ b/rmgpy/tools/data/diffmodels/species_dictionary3.txt @@ -0,0 +1,42 @@ +ethane +1 C 0 0 {2,S} {3,S} {4,S} {5,S} +2 C 0 0 {1,S} {6,S} {7,S} {8,S} +3 H 0 0 {1,S} +4 H 0 0 {1,S} +5 H 0 0 {1,S} +6 H 0 0 {2,S} +7 H 0 0 {2,S} +8 H 0 0 {2,S} + +CH3 +1 C 1 0 {2,S} {3,S} {4,S} +2 H 0 0 {1,S} +3 H 0 0 {1,S} +4 H 0 0 {1,S} + +C2H5 +1 C 0 0 {2,S} {3,S} {4,S} {5,S} +2 C 1 0 {1,S} {6,S} {7,S} +3 H 0 0 {1,S} +4 H 0 0 {1,S} +5 H 0 0 {1,S} +6 H 0 0 {2,S} +7 H 0 0 {2,S} + +C +1 C 0 0 {2,S} {3,S} {4,S} {5,S} +2 H 0 0 {1,S} +3 H 0 0 {1,S} +4 H 0 0 {1,S} +5 H 0 0 {1,S} + +CH2O(150) +1 C u0 p0 c0 {2,D} {3,S} {4,S} +2 O u0 p2 c0 {1,D} +3 H u0 p0 c0 {1,S} +4 H u0 p0 c0 {1,S} + +H(6) +1 O 0 2 {2,S} {3,S} +2 H 0 0 {1,S} +3 H 0 0 {1,S} \ No newline at end of file diff --git a/rmgpy/tools/merge_modelsTest.py b/rmgpy/tools/merge_modelsTest.py new file mode 100644 index 0000000000..16a59a4cf7 --- /dev/null +++ b/rmgpy/tools/merge_modelsTest.py @@ -0,0 +1,84 @@ +#!/usr/bin/env python +# -*- coding: utf-8 -*- + +############################################################################### +# # +# RMG - Reaction Mechanism Generator # +# # +# Copyright (c) 2002-2019 Prof. William H. Green (whgreen@mit.edu), # +# Prof. Richard H. West (r.west@neu.edu) and the RMG Team (rmg_dev@mit.edu) # +# # +# Permission is hereby granted, free of charge, to any person obtaining a # +# copy of this software and associated documentation files (the 'Software'), # +# to deal in the Software without restriction, including without limitation # +# the rights to use, copy, modify, merge, publish, distribute, sublicense, # +# and/or sell copies of the Software, and to permit persons to whom the # +# Software is furnished to do so, subject to the following conditions: # +# # +# The above copyright notice and this permission notice shall be included in # +# all copies or substantial portions of the Software. # +# # +# THE SOFTWARE IS PROVIDED 'AS IS', WITHOUT WARRANTY OF ANY KIND, EXPRESS OR # +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, # +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE # +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER # +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING # +# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER # +# DEALINGS IN THE SOFTWARE. # +# # +############################################################################### + +import unittest +import os +import os.path + +from rmgpy.tools.merge_models import get_models_to_merge, combine_models + + +class MergeModelsTest(unittest.TestCase): + + def test_merge_different_models(self): + folder = os.path.join(os.getcwd(), 'rmgpy/tools/data/diffmodels') + + chemkin3 = os.path.join(folder, 'chem3.inp') + speciesDict3 = os.path.join(folder, 'species_dictionary3.txt') + + chemkin2 = os.path.join(folder, 'chem2.inp') + speciesDict2 = os.path.join(folder, 'species_dictionary2.txt') + + models = get_models_to_merge(((chemkin3, speciesDict3, None), (chemkin2, speciesDict2, None))) + final_model = combine_models(models) + species = final_model.species + reactions = final_model.reactions + + # make sure all species are included + self.assertEqual(len(species), 15) + + # make sure indexes are not unnecessarily redone + for s in species: + if s.label == 'CH2O': + self.assertEqual(s.index, 150) + elif s.label == 'CH3': + self.assertEqual(s.index, -1) + elif s.label == 'C3H7': + self.assertEqual(s.index, 14) + + # make sure indexes are redone when there is a conflict + H_index = False + for s in species: + if s.label == 'H': + if isinstance(H_index, bool): + H_index = s.index + else: + # found second matching label, make sure index different + self.assertNotEqual(s.index, H_index) + break + else: + raise Exception("Could not find two species identical labels") + + # make sure reaction rates come from first model + for r in reactions: + if len(r.reactants) == 2 and r.reactants[0].label == 'CH3' and\ + r.reactants[1].label == 'CH3': + self.assertAlmostEqual(r.kinetics.A.value_si, 8.260e+9, places=0, + msg="Kinetics did not match from first input model") From a4f0320ae12e15e746ba66eade893ecd79e73c66 Mon Sep 17 00:00:00 2001 From: MarkGoldman Date: Fri, 5 Jul 2019 11:51:28 -0400 Subject: [PATCH 4/4] Minimize reindexing in merge_models This commit replaces a complete reindex for merged models with a smart (as in 'smart speaker') algorithm that only reindexes a species when it is has both an identical label and index with another species. This prevents conflicts and reduces unnecessary reindexing. --- rmgpy/rmg/model.py | 9 +-------- rmgpy/tools/merge_models.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/rmgpy/rmg/model.py b/rmgpy/rmg/model.py index 18be219271..c0264817ea 100644 --- a/rmgpy/rmg/model.py +++ b/rmgpy/rmg/model.py @@ -118,14 +118,7 @@ def merge(self, other): # Add the unique species from other to the final model finalModel.species.extend(uniqueSpecies) - - # Renumber the unique species (to avoid name conflicts on save) - speciesIndex = 0 - for spec in finalModel.species: - if spec.label not in ['Ar','N2','Ne','He']: - spec.index = speciesIndex + 1 - speciesIndex += 1 - + # Make sure unique reactions only refer to species in the final model for rxn in uniqueReactions: for i, reactant in enumerate(rxn.reactants): diff --git a/rmgpy/tools/merge_models.py b/rmgpy/tools/merge_models.py index c59790aa90..90200c29f1 100644 --- a/rmgpy/tools/merge_models.py +++ b/rmgpy/tools/merge_models.py @@ -136,6 +136,7 @@ def get_models_to_merge(input_model_files): def combine_models(models): """ Takes in a list of ReactionModels and and merges them into a single ReactionModel + Reindexes species with the same label and index """ final_model = ReactionModel() for i, model in enumerate(models): @@ -160,4 +161,17 @@ def combine_models(models): '#{0:d}.'.format(i+1, Nrxn - Nrxn0, len(model.reactions))) print('The merged model has {0:d} species and {1:d} reactions' ''.format(len(final_model.species), len(final_model.reactions))) + + # ensure no species with same name and index + label_index_dict = {} + for s in final_model.species: + if s.label not in label_index_dict: + label_index_dict[s.label] = [s.index] + else: + if s.index in label_index_dict[s.label]: + # obtained a duplicate + s.index = max(label_index_dict[s.label]) + 1 + print("Reindexed {0} due to dublicate labels and index".format(s.label)) + label_index_dict[s.label].append(s.index) + return final_model