mtholder / ncl

Nexus Class Library
GNU General Public License v2.0
8 stars 16 forks source link

Duplicate node ids created (possibly due to multiple trees?) #2

Closed jimallman closed 10 years ago

jimallman commented 10 years ago

I'm calling NCLconverter from the Open Tree curation app, sending the test file avian_ovomucoids.nex to import its trees. It's then called with these options:

['/Users/jima/.virtualenvironments/opentree/bin/python',
 '/Users/jima/projects/nescent/opentree/web2py/applications/curator/modules/joblauncher.py',
 '/Users/jima/projects/nescent/opentree/opentree/curator/private/scratch/2nexml/uef741a68-912f-4e8c-ba8b-0cfdf08843ba',
 '',
 'out.xml',
 'err.txt',
 '/Users/jima/projects/nescent/opentree/opentree/curator/private/bin/NCLconverter',
 '-fnexus',
 '-g',
 '-pe505',
 '-pO10',
 '-pt3',
 '-pT10',
 '-po128',
 '-pn505',
 'in.nex']

Note that the next available node id is 505. When the NexSON is returned with added trees, there are dozens (not hundreds) of duplicate ids: 505, 506, 509, 636, ... reported in the JS console.

Could the duplication be happening because there are multiple trees in this file?

mtholder commented 10 years ago

oops. I thought that I had tested that. I'll look into it today. Should be a quick fix (famous last words)

mtholder commented 10 years ago

Fixed. Thanks Jim, that was a scary bug (see https://github.com/mtholder/ncl/commit/6fb4c1895bc222a696f1050386e51a6862898b1a for details) that would lead to undefined behavior. Introduced when I started supporting user-specification of NeXML ids counter.

For other NCL users: This bug only affect writing of NeXML. No other NCL functions were involved. So this was a bug in the example code, not the library part of NCL.