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

make ck2cti more deterministic in it's output #497

Merged
merged 2 commits into from
Jan 24, 2018

Conversation

skyreflectedinmirrors
Copy link
Contributor

A few simple fixes (sorting of dictionary items) to ensure that ck2cti's output is more deterministic.

The context of this is I have a test mechanism I have in chemkin format, and a (hopefully) identical mechanism in cantera format.

In pyJac we have our own interpreter that can parse either format, so we have unit tests that
1.) Ensure that our own internal data representations of both the parsed chemkin and cantera mechanisms are identical.
2.) Test that converting the chemkin model to cantera format produces the same mechanism as we have stored in our repository -- this is easiest to accomplish via difflib or the like (rather than writing equality functions for Cantera reaction/species classes, although that might be a worthwhile endeavor at some point)

The current issue I'm facing is that often we see different outputs from ck2cti, for example

species(name=u'OH',
        atoms='H:1 O:1',

vs.

species(name=u'OH',
        atoms='O:1 H:1',

IMO the (minor) performance hit of sorting the dictionaries based on the items (effectively, sorting based on the key) is worth having consistent deterministic output :)

Copy link
Member

@speth speth 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 me.

@speth speth merged commit 411be3e into Cantera:master Jan 24, 2018
@skyreflectedinmirrors skyreflectedinmirrors deleted the ck2cti_deterministic branch January 24, 2018 15:23
srikanthallu pushed a commit to srikanthallu/cantera that referenced this pull request Sep 17, 2020
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.

None yet

2 participants