search

IUPAC-InChI / InChI

Main InChI repository
MIT License
60 stars 7 forks source link

MOBILE_H syntax error observed with v1.07 I/O validation #43

Open baleland opened 1 month ago

baleland commented 1 month ago 
CMD> inchi-1.exe ptable.sdf /FixedH /RecMet
CMD> inchi-1.exe ptable.sdf.txt /InChI2InChI /FixedH /RecMet

results in,

Structure: 1 Syntax error (-2) in MOBILE_H, Reconnected layer (102)

Structure: 2 Syntax error (-2) in MOBILE_H, Reconnected layer (102)

Structure: 3 Syntax error (-2) in MOBILE_H, Reconnected layer (102)

I am loathe to restandardize tests generally w/o a firm understanding. Here the inchi utility itself is emitting output that cannot be reparsed w/o error, which seems generally wrong.

This is a test standard we have run successfully v1.05 & v1.06, which now fails.

ptable.sdf.txt

djb-rwth commented 1 month ago

Hi @baleland, I can confirm this output and will mark it as a bug. Will get back with the fix shortly.

djb-rwth commented 1 month ago

Hi @baleland, I think I am very near fixing this issue. Please let me know what is your deadline and if you think that this fix might resolve the testing problems you are facing.

baleland commented 1 month ago

we are in the beta cycle, so probably ~1mo away from a release candidate lockdown. while this fix (& the others) would be required, there is still the issue(s) of a number of tests that differ from v1.06 - unclear that we would move to v1.07.1/2 unless those differences are fully understood.

djb-rwth commented 1 month ago

Hi @baleland, Thanks for the information. I think I might have fixed this issue as well. Please swap the ichiread.c with the one attached to this message and do let me know if this issue has been fixed. The problem was similar to the one we experienced in issue #27 and #28. ichiread.c.txt

baleland commented 1 month ago

much more progress made now!! All the InChI tests were now able to be run.
Only ONE test "fails" due to differences b/w the v1.06 and v1.07.1.x results - maybe due to needing restandardization, or maybe due to and actual/unintended difference,

***KNOWN FAILURES: 0
*** NEW  FAILURES: 262

would you like me to hand select some representative examples from these 262 failures and open a new case for analysis/verifcation?

djb-rwth commented 1 month ago

much more progress made now!! All the InChI tests were now able to be run. Only ONE test "fails" due to differences b/w the v1.06 and v1.07.1.x results - maybe due to needing restandardization, or maybe due to and actual/unintended difference,

Hi @baleland, Thanks for the great news.

***KNOWN FAILURES: 0
*** NEW  FAILURES: 262

would you like me to hand select some representative examples from these 262 failures and open a new case for analysis/verifcation?

Yes, absolutely. I will close this issue and you can open a new one now.

baleland commented 1 month ago 
InChI_1_07_0\bugfix\inchi-1.exe STATUS_7_IO_ERROR.txt /InChI2InChI /FixedH /RecMet

! Working in engineering mode

Structure: 1 Syntax error (-2) in MOBILE_H (2)
Finished processing 1 structure: 0 errors, processing time 0:00:00.00

we see 25 I/O failures of this type in our tests (with your 2x module updates) - the bugfix exe you provided previously does not have your latest fixes.

STATUS_7_IO_ERROR.txt

baleland commented 1 month ago

similarly for this set of structures, we see 237 I/O failures. I do not have an inchi-1.exe with your updates to compare with, but there are at least 59 errors from the last version you provided.

STATUS_8_EXAMPLES.txt

djb-rwth commented 4 weeks ago

Hi @baleland, I reopened this issue so we can take it from here. Will get back to you shortly.

djb-rwth commented 4 weeks ago

Hi @baleland, It looks as if all these tests have passed with the fixes I applied above. I am sending you an .exe version for Windows and elf file for Ubuntu/Linux in the attachment along with the test results. Please let me know if the tests pass with these executables at your end. If they do, I will upload a new version to our rwth branch, i.e. both the source codes and the executables. v_1.07.2_rc.zip

baleland commented 4 weeks ago

Ok, I guess I'll have to walk through some of the test failures to see why the test results are different than the inchi-1.exe I/O.

baleland commented 4 weeks ago

Ok - I've done a thorough review of our existing tests. One particular test (rarely run) "appeared" to show a difference b/w v1.06 and v1.07 I/O round-trip testing via /FixedH /RecMet.

In reality, it appears this test had been disabled and the v1.06 output has the same number of I/O errors as v1.07. The errors themselves are likely due to our testing code, nothing to do with the InChI engine itself, seemingly.

djb-rwth commented 3 weeks ago

Hi @baleland, Thank you for the report. Please do keep me updated on this.

flange-ipb commented 1 week ago

Thomas Exner submitted a very similar bug report that is resolved by the patched executables provided by @djb-rwth in https://github.com/IUPAC-InChI/InChI/issues/43#issuecomment-2289515139.

inchi-1 input.inchi -InChI2Struct -OutputSDF fails with 

Structure: 1 Syntax error (-2) in MOBILE_H (2)

for the input InChI InChI=1S/C6H8O7.3Na.2H2O/c7-3(8)1-6(13,5(11)12)2-4(9)10;;;;;/h13H,1-2H2,(H,7,8)(H,9,10)(H,11,12);;;;2*1H2/q;3*+1;;/p-3 (Sodium citrate (dihydrate)).