bugGNU Octave - Bugs: bug #51512, [octave forge] (io) Missing or...

 
 

bug #51512: [octave forge] (io) Missing or wrong types when using xlsread with OCT interface

Submitted by:  Markus Mützel <mmuetzel>
Submitted on:  Wed 19 Jul 2017 09:32:10 PM UTC  
 
Category: Octave Forge PackageSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: Ready For TestAssigned to: Philip Nienhuis <philipnienhuis>
Originator Name: Open/Closed: Open
Release: otherOperating System: Any

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Sun 13 Aug 2017 03:29:54 PM UTC, comment #19:

The last version seems to run at more or less the same speed as the version before.
Your file (file #41434):

My file (file #41352 + changes from comment #16):

Immediately followed by "COM":

I could not find any reference to a date/time type in the specifications. The only available types I found are:

Dates are stored as doubles in my tests. But I suppose it doesn't hurt to leave the logic in if you have seen that type in some file.

In the attached file, I tried to replace any "." match with a negative match to avoid the trouble with PCRE's MATCH_LIMIT. Additionally, I changed slightly how the matched results are checked for consistency such that multiples of 3 errors can also be detected.
The file still performs approximately identically:

(file #41505)

Markus Mützel <mmuetzel>
Project Member
Sun 06 Aug 2017 01:58:03 AM UTC, comment #18:

-file got lost in preview, attempt #2-

(file #41434)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Sun 06 Aug 2017 01:56:12 AM UTC, comment #17:

Thanks Markus,

Attached is yet another version (perhaps a bit incomplete) in which I tried to cut down on the number of different regexps.

However, on my laptop (which BTW is 2 X slower than my desktop) it didn't make much difference reading the file in comment #14, it was ~128 secs (version of comment #13) versus ~125 secs. (this version). Inside profshow's results there wasn't much difference either. Apparently there's little to be gained by combining regexps, it seems to be the total complexity that counts.
Maybe you can experiment with it a little if you want.

From tomorrow on I'll have patchy internet access for 2 weeks so expect little or no sign of life from me until Aug. 21 or so.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Wed 02 Aug 2017 08:48:26 PM UTC, comment #16:

It definitely is an improvement if more data can be fetched. And you could reduce code complexity at the same time. Excellent.

I had problems reading a big file with approximately 1000x1000 cells saved with Excel 2016 that contains doubles, Booleans, and strings (no dates) as values and as formula results. (That file did not upload the last time, probably due to file size limits. File size 8 MB. xml string size 44.2 MB.)
With Octave 4.2.1 on Windows, it gave the following warning and then caused a segfault:

The help string for "regexp" was updated recently by Dan Sebald regarding this issue.

Additionally, a formula can span several rows. I didn't realize that until now. In this case, a cell could be described like this in xml:

Note that the f-tag has no content. That isn't yet matched by the regexp in the file from comment #13.
I tried with the following regexp in the failing line:

I don't know whether this is the most efficient regular expression. But it doesn't segfault for me and matches all strings.
Performance-wise:

Compared to "COM" (Excel 2016):

Excel (via COM) is still 16x faster for me. But you should probably compare with the same file on the same system to get comparable results. I can send you the file I used for testing by email if you would like to test with that one.
My system: Windows 10 Home 1703, Core i7 7500U @ 2.7 GHz, 8 GB RAM, Octave installed on a SSD but reading the test file from a HDD.

Markus Mützel <mmuetzel>
Project Member
Fri 28 Jul 2017 06:26:35 PM UTC, comment #15:

Comparing with original io-2.4.7, reading the 1000x1000 spreadsheet using OCT takes only 60 secs. But then there wasn't a separate regexp for boolean (logical) data, nor for "inlineStr" string data.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Fri 28 Jul 2017 06:10:54 PM UTC, comment #14:

Some timings on my Core Duo i5 box (Win 7 Prof., 8 GB RAM & SSD):

So COM (w. Excel 2013 as work horse) is by far the fastest, UNO (invoking LibreOffice) by far the slowest, and OCT some 25 X slower than COM.
In a way disappointing that proprietary SW is so much faster, but on the bright side: OCT is still 2nd fastest.

Some profiling:

I tried experimenting with preallocating regexp's output array but that didn't help much.
These profiling results show that tuning the regexp, if at all possible, is probably the best way to get better performance.
Maybe the modification I made (obtaining the cell addresses and contents in one regexp) is slower than getting them separately with two regexps as in the original __OCT_xlsx2oct__.m .
Note: in this case (for a 1000x1000 array) the XML string to be processed is about 37 MB in size.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Wed 26 Jul 2017 09:35:20 PM UTC, comment #13:

Markus,

Attached is a patch that AFAICS should work fine now. I also had to fix __OCT_oct2xlsx__.m to beter comply with the OOXML specs.
Could you please try this patch and report back?
To make it easier I also attached the fixed __OCT_oct2xlsx__.m and __OCT_xlsx2oct__.m files, you can just swap them into the io/../private subdir.

I wouldn't mind if you (or someone else) improve the patch. I'll see what it looks like when I'm back late August, then hopefully it has been tested / tested+improved sufficiently so that the final fix can be pushed.

Thanks

(file #41350, file #41351, file #41352)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 25 Jul 2017 08:10:00 PM UTC, comment #12:

(progress report)
I started overhauling most of the regular expressions.

At this point all numeric & boolean contents are catched correctly; I'm presently stuck in the strings dept. It seems inline strings were never processed correctly so there's an improvement looming.

It seems we now also have an option to correctly extract date/time values.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 25 Jul 2017 12:37:18 PM UTC, comment #11:

(found some spare time)
Comparing LO53's sheet1.xml and Excel2013's sheet1.xml it turns out that Excel writes a different attribute order than LO.
Luckily the 'r=' (cell address) and 't=' tags are always in that order (r first).

Experimenting further I saw that probably half of the regexps can be dropped as cell addresses and cell content can be obtained in one regexp and separated using reshape. Some additional checks are required to avoid out-of-phase matches (which would be killing for reshape). Maybe I can finish this this week, or maybe next week. After Aug 6 I'm 2 week w/o much web access.

Anyway I'm happy that the xlsx read part of the OCT interface gets some attention.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 25 Jul 2017 06:11:19 AM UTC, comment #10:

Ah boolean and double regexps reversed :-)
(I need coffee)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 25 Jul 2017 06:10:36 AM UTC, comment #9:

Singling out booleans as you do is the way to go, yes.

Cf. ECMA specs 's="<number>"' tags can be ignored as they just refer to styles. So a regexp along the way of

should work better. The "(?:.......)" clauses (for tags that may not be present) need attention. I've experimented a little yesterday evening, but this has to be sorted out more rigorously.
A flowchart is needed to decide what data to read/tags to search for in what order. Some regexps may even be dropped, hopefully leading to faster reading.
For reference I've attached worksheet1.xml from the files made by Excel2013 and LO5.2. What I usually do is just read them with fread(<fid, Inf, "char=>char")' as text strings into the Octave terminal and then experiment with regexp() until I get it right.

I think what Excel writes is still the reference (as that's the file "format" most users will encounter); LO / OOo is not completely ECMA-compliant.

The timings you mention aren't that bad. Have you compared with Excel itself (just reading, not tru COM interface per se)?

(file #41326, file #41327)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Mon 24 Jul 2017 08:54:54 PM UTC, comment #8:

Philip,

I timed different variations of the patch while reading the attached file with roughly 1000x1000 cells.
With the current version of _OCT_xlsx2oct_.m: 79.3s
With the "cellfun patch" from comment #1: 108.5s
With a dedicated regexp for reading Booleans (patch attached to this comment): 92.6s

However, most of the additional time is spend reading the strings that the current version skips. Without that particular change (optional s-tag), the time reduced to: 79.2s.

So there is hardly any reduction in speed (if any at all), when taking the number of cells into account that are read as non-empty.

(file #41322)

Markus Mützel <mmuetzel>
Project Member
Mon 24 Jul 2017 07:47:28 PM UTC, comment #7:

Markus,

The OOXML specs are here:
https://www.ecma-international.org/publications/standards/Ecma-376.htm

Looking through it I think several regexps in _OCT_xlsx2oct_.m may have to be reconsidered. Once done there's no need for helper subfunctions. I know what to do but the next weeks I lack time, hopefully I can pick that up after my vacation, or maybe you or someone else will beat me to it.
Now that I mention: I remember to have struggled a bit with it before; at the time I found Excel itself wasn't always complying to the ECMA rules and Libre-/OpenOffice still less.

I agree LibreOffice is a bit lacking. You may want to file an upstream bug report about setting the result type of boolean formulas to "numeric". The folks at LibreOffice seem relatively responsive.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Mon 24 Jul 2017 02:28:15 PM UTC, comment #6:

OK, 10,000 x 1000 may be a bit excessive; that's 80+ MB just for the double array, and maybe tens of times times bigger for the XML text string to be read.

1000 x 1000 should do.

As to the LibreOffice quirks (the same as OpenOffice.org quirks) you mention in the bottom of your original report:
In the UNO interface there's no dedicated ID for boolean values, AFAIU because the LO/OOo code base doesn't contain it, so there's no way (at least not that I know) to find out the type/class of data in a spreadsheet cell. That's quite a bit of a limitation if you ask me; IIRC I may even have filed a bug report for that long ago (~2011 or 2012) for OOo.
To show what I mean, see L.50-55 and L.102-109 in _UNO_spsh2oct_.m and here:
https://www.openoffice.org/api/docs/common/ref/com/sun/star/sheet/CellFlags.html

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Fri 21 Jul 2017 06:02:41 PM UTC, comment #5:

Not even that, but regexps can be tuned as well. Speed differenes between slow and fast regexps can be up to tens of percents.
There's a bit of hidden literature bout it on the web and I think Rik knows a lot about it.

I think the cellfun stuff you wrote could be much faster if the actual contents (is_bool_value) were written in C++

Be sure to benchmark with large spreadsheets (10,000 rows by -say- 1000 columns or so).

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Fri 21 Jul 2017 05:32:05 PM UTC, comment #4:

Philip,

I did not expect that "regexp" would be faster on long inputs than a call to "cellfun" with rather simple functions. But preliminary test indicate that "regexp" is impressively fast.
I will prepare an adapted changeset and try to upload it maybe early next week together with some benchmark results if possible.

Markus Mützel <mmuetzel>
Project Member
Fri 21 Jul 2017 07:41:41 AM UTC, comment #3:

Markus, I'm busy with it; may take some time.

I'm not surprised LibreOffice (and for that matter, OpenOffice.org) and Excel write the OOXML differently, that was a quirk I had to code around early on. That's one of the tricky things when reading XML with regexps rather than with an XML parser - XML gurus tend to frown upon that. This is a major weak point in the OCT spreadsheet interface.

I see a new subfunc "is_bool_value". I'd like to find out if adapting regexps is faster.
The OCT interface used to be blazingly fast for OOXML, much faster than Excel and Matlab, but adding more and more type/subtypes (than just double) and Excel-quirk-workarounds made it less efficient.
So I think the way this should be fixed (regexp, or postprocessing regexp output in a separate subfunc, or some other way) should depend on timings with large test spreadsheets, ideally with mixed types.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Thu 20 Jul 2017 09:57:52 AM UTC, comment #2:

Thx I'll have a look

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Wed 19 Jul 2017 09:40:43 PM UTC, comment #1:

Here the patch.

(file #41256)

Markus Mützel <mmuetzel>
Project Member
Wed 19 Jul 2017 09:32:10 PM UTC, original submission:

Attached are four example xlsx files created with Excel 2007, Excel 2013, LibreOffice 5.2 and LibreOffice 5.3.
When reading from the Excel files, the cells that contain string formula results are read as empty. The cells that contain Boolean values or formulas are read as numerics.
When reading from the LibreOffice files, the cells that contain formulas with numeric or logical results are read as strings. Boolean values are read as numeric.

The following script displays the issues:

I will attach a patch that fixes these issues as soon as I have a bug number.
I could not figure out how to read Boolean values from formulas correctly from the LibreOffice files. But I am inclined to blame LibreOffice since they tag these cells as numeric (t="n") instead of Boolean (t="b"). The cells even show up incorrectly (numeric instead of logical) when opening the files in LibreOffice. So xlsread will read these cells as numeric, too. (Better than as strings imho.)

Markus Mützel <mmuetzel>
Project Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #41505:  __OCT_xlsx2oct__.m added by mmuetzel (10KiB - application/octet-stream)
file #41434:  __OCT_xlsx2oct__.m added by philipnienhuis (10KiB - d2l/unknowntype)
file #41350:  bug_#51512_PRN.patch added by philipnienhuis (15KiB - application/x-download)
file #41351:  __OCT_oct2xlsx__.m added by philipnienhuis (20KiB - text/x-objective-c)
file #41352:  __OCT_xlsx2oct__.m added by philipnienhuis (10KiB - text/x-objective-c)
file #41322:  bug51512_xlsx2oct_types_v2.patch added by mmuetzel (7KiB - application/octet-stream)
file #41256:  bug51512_xlsx2oct_types.patch added by mmuetzel (6KiB - application/octet-stream)
file #41252:  Excel2007_test.xlsx added by mmuetzel (8KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #41253:  LibreOffice52_test.xlsx added by mmuetzel (5KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #41254:  Excel2013_test.xlsx added by mmuetzel (8KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
file #41255:  LibreOffice53_test.xlsx added by mmuetzel (5KiB - application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jwe (Updated the item)
  • -unavailable- added by mmuetzel (Submitted the item)
  • -unavailable- added by mmuetzel
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 19 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 13 Aug 2017 03:29:54 PM UTCmmuetzelAttached File-=>Added _OCT_xlsx2oct_.m, #41505
    Sat 12 Aug 2017 12:39:02 AM UTCjweSummaryof-io: Missing or wrong types when using xlsread with OCT interface=>[octave forge] (io) Missing or wrong types when using xlsread with OCT interface
    Sun 06 Aug 2017 01:58:03 AM UTCphilipnienhuisAttached File-=>Added _OCT_xlsx2oct_.m, #41434
    Wed 26 Jul 2017 09:35:20 PM UTCphilipnienhuisAttached File-=>Added _OCT_xlsx2oct_.m, #41352
      StatusIn Progress=>Ready For Test
    Wed 26 Jul 2017 09:35:19 PM UTCphilipnienhuisAttached File-=>Added bug_#51512_PRN.patch, #41350
      Attached File-=>Added _OCT_oct2xlsx_.m, #41351
    Tue 25 Jul 2017 08:10:00 PM UTCphilipnienhuisStatusPatch Submitted=>In Progress
    Tue 25 Jul 2017 06:10:36 AM UTCphilipnienhuisAttached File-=>Added Ex13_sheet1.xml, #41326
      Attached File-=>Added lo52_sheet1.xml, #41327
    Mon 24 Jul 2017 08:54:54 PM UTCmmuetzelAttached File-=>Added bug51512_xlsx2oct_types_v2.patch, #41322
    Thu 20 Jul 2017 09:57:52 AM UTCphilipnienhuisAssigned toNone=>philipnienhuis
    Wed 19 Jul 2017 09:40:43 PM UTCmmuetzelAttached File-=>Added bug51512_xlsx2oct_types.patch, #41256
      StatusNone=>Patch Submitted
    Wed 19 Jul 2017 09:32:10 PM UTCmmuetzelAttached File-=>Added Excel2007_test.xlsx, #41252
      Attached File-=>Added LibreOffice52_test.xlsx, #41253
      Attached File-=>Added Excel2013_test.xlsx, #41254
      Attached File-=>Added LibreOffice53_test.xlsx, #41255
      Carbon-Copy-=>Added philipnienhuis

    Back to the top


    Powered by Savane 3.1-cleanup1