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

Need a check to ensure children are actually children of the parent node #185

Closed
shamelmerchant opened this issue Feb 17, 2014 · 6 comments

Comments

@shamelmerchant
Copy link
Contributor

With the introduction of nitrogen and sulfur we have missed adding the hetroatoms in the parent nodes but define children nodes specific to N and S. This then results in the rate estimation just falling up to the head node and never descending down the tree.

Connie also sees other examples where this happens.

@bbuesser
Copy link
Contributor

Where does it happen for nitrogen?

@shamelmerchant
Copy link
Contributor Author

An example which I found was in H abstraction

I was trying to add a rate rule for L5: Cd/H2/NonDeN, H_rad but it kept on hitting X_H, H_rad because the parent two level up L3: Cd_H was only defined for C=C and not {C,N}.

@rwest
Copy link
Member

rwest commented Feb 17, 2014

I suggest:

  1. get Database.isWellFormed() working
  2. call isWellFormed() after loading (or importing) each database (maybe append to the load functions)
  3. add a check to the end of isWellFormed() that ensures each node's parent is indeed a parent. You should be able to use the isSubgraphIsomorphic() method on each entry.
  4. create some unit tests for all of the above!

@bbuesser
Copy link
Contributor

@rwest In the comment of Database.isWellFormed() I have found a sentence I don't understand:

If no tree is present (e.g. the primary libraries), then every entry in the library must have an entry in the dictionary.

Is it referring to a Java-style library?

@bbuesser
Copy link
Contributor

Oh, now I see how it works.

@connie
Copy link
Member

connie commented Oct 21, 2014

This is now part of a unit test suit within RMG-Py/databaseTest.py

@connie connie closed this as completed Oct 21, 2014
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

No branches or pull requests

4 participants