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

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

Open
baleland opened this issue Aug 12, 2024 · 19 comments
Open

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

baleland opened this issue Aug 12, 2024 · 19 comments
Assignees
Labels
bug Something isn't working

Comments

@baleland
Copy link

baleland commented Aug 12, 2024

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
Copy link
Collaborator

djb-rwth commented Aug 13, 2024

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

@djb-rwth djb-rwth self-assigned this Aug 13, 2024
@djb-rwth djb-rwth added the bug Something isn't working label Aug 13, 2024
@djb-rwth
Copy link
Collaborator

djb-rwth commented Aug 13, 2024

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

baleland commented Aug 13, 2024

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
Copy link
Author

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 djb-rwth reopened this Aug 14, 2024
@djb-rwth
Copy link
Collaborator

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

@djb-rwth
Copy link
Collaborator

djb-rwth commented Aug 14, 2024

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
Copy link
Author

baleland commented Aug 14, 2024

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
Copy link
Author

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
Copy link
Collaborator

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

@flange-ipb
Copy link
Collaborator

Thomas Exner submitted a very similar bug report that is resolved by the patched executables provided by @djb-rwth in #43 (comment).

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)).

@djb-rwth
Copy link
Collaborator

Hi @flange-ipb,
Thank you for confirming this.
In that case, it seems that we have fixed all major bugs which will be added in v1.07.2.

Still waiting for @baleland's confirmation that there are no new issues at his end.

@baleland
Copy link
Author

baleland commented Sep 21, 2024 via email

@djb-rwth
Copy link
Collaborator

Hi @baleland,
All fixes to issues we discussed in your posts have been integrated in InChI v1.07.2, which has now been uploaded to rwth branch.
Please feel free to let me know if you have any further suggestions or comments in this regard.
Hoping to hear from you soon.

@baleland
Copy link
Author

baleland commented Nov 12, 2024 via email

@djb-rwth
Copy link
Collaborator

Hi @baleland,
No problems -- will let you know as soon as v.1.07.2 release candidate becomes an official release, i.e. is uploaded to main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants