bugGNU Octave - Bugs: bug #66790, [octave-forge] (io) Failing tests...

 
 

bug #66790: [octave-forge] (io) Failing tests with Octave 10 in toJSON.m and fromJSON.m

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Wed 12 Feb 2025 09:35:49 AM UTC
   
 
Category:  Octave Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error or Warning
Status:  In Progress Assigned to:  philipnienhuis
Originator Name:  Open/Closed:  * Open
Release:  * 10.0.90 Release: 
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 07 Mar 2025 06:20:33 PM UTC, comment #15: 

Again I probably misunderstood something, but on octave 10.0.90 with current io snapshot I got:

***** test  ## vectors
  assert(toJSON([1,2,3]),'[1,2,3]')
  assert(toJSON([1;2;3]),'[1,2,3]')
!!!!! test failed
ASSERT errors for:  assert (toJSON (([1, 2, 3])),'[1,2,3]')

  Location  |  Observed  |  Expected  |  Reason
     []      '[[1,2,3]]'   '[1,2,3]'     Strings don't match


(there are 8 errors total)

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Mon 03 Mar 2025 07:34:56 PM UTC, comment #14: 

toJSON.m fix for some cleaner code pushed here:
http://hg.code.sf.net/p/octave/io/rev/b2c7086e7324
I didn't amend the warning itself (as suggested in comment #23 of bug #41028) as that FAIL was due to a "regression" fixed in the course of that bug.

Next job: fromJSON.m.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 26 Feb 2025 11:38:20 AM UTC, comment #13: 


> which is clever but may take a lot of memory

Aha, yeah I've seen that over and over again during my career: the unfounded assumption that compact and convoluted code by definition equates to faster and better code :-)
Often, apart from taking longer to execute, it also takes longer for outsiders to fully grasp what is going on ...

That said, I'd immediately want to reiterate here that IMO the author of the toJSON and fromJSON functions did a great job at the time; creating and esp. parsing JSON is hard. I tried to fix some of the BIST FAILs in fromJSON.m but soon got lost.

Thanks for the help, appreciated.
If you get inspired, over in patch #9980 there are several more JSON files and URLs to large JSON files to play with.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 25 Feb 2025 10:05:52 PM UTC, comment #12: 

I did some profiling with JSON files mentioned in comment 3.
The bottleneck seems to be the function bfilter and in particular the line "idx( any(qo < ai & ai < qc,1) ) = [];",
which is clever but may take a lot of memory (qo < ai essentially builds a matrix of length(q0)*length(ai)). So I did a purely loop based approach and it turns up surprisingly fast for even mediaum-size records (i.e. less than computer memory). It looks like binary "&" is quite slow for large arrays. Anyway, the revised
function is attached (replace the one in "fromJSON.m").
With it I get:

octave:1> jstr = fileread("citm_catalog.json") ;
octave:2> tic; x0 = fromJSON(jstr); toc
Elapsed time is 11.654 seconds.

(41.5s with the original code).
Also it can now read "large-file.json" w/o crashing (though it took few hours.

Dmitri.
--



(file #56952)

Dmitri A. Sergatskov <dasergatskov>
Tue 25 Feb 2025 11:49:03 AM UTC, comment #11: 

Dmitri,

In fact this bug report is exactly about test failures with toJSON.m and fromJSON.m, so let's keep things simple and discuss it here? I'll adapt the title a little.
If you have some improvements, good, thanks and please put them up here :-)

Or would you want to discuss the test failures from a more general POV that would better fit at Discourse?

Just a note:
At the time I had a discussion with the author about a.o., coding style, but he insisted. Yet the functions seemed useful to me, e.g., they accept complex variables which Matlab doesn't do (for the moment ignoring whether that's useful at all), so I accepted them with the provision that I wouldn't maintain them.
So now I'm feeling a little itchy because not only me but also other people are putting effort in them ...

Philip Nienhuis <philipnienhuis>
Group Member
Tue 25 Feb 2025 03:14:31 AM UTC, comment #10: 

Philip,

I played a little with the "fromJSON" code and I think I can offer some improvements. Do you want to open a dedicated bug report or would you prefer that discussion to be on discourse?

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Tue 25 Feb 2025 12:27:37 AM UTC, comment #9: 

Philip, it is OK with me. I was worried that I am tracking a wrong repo.

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Tue 25 Feb 2025 12:19:28 AM UTC, comment #8: 

With "attach it to this bug report" I mean toJSON.m

Philip Nienhuis <philipnienhuis>
Group Member
Tue 25 Feb 2025 12:18:05 AM UTC, comment #7: 

Hi Dmitri:
For you nothing tangible has been fixed. I had other things on my hands (real life) the last week :-)
I just fixed toJSON.m in my local repo, for a final fix you'll have to wait until the next io release. Later this week I'll see if I can attach it to this bug report.

Fixing fromJSON.m is much more work. Code quality leaves much to be desired, apart from more philosophical things, see over at bug #41028. Don't count on getting it all done within the next few weeks.
Maybe I'll just disable the failing tests for now.

A new io release depends on quite a few other issues, a.o., Java changes for the various spreadsheet I/O interfaces. A new thing is that they now seem to interfere with each other, that didn't happen until a while ago. S/th with namespaces / Java 'import' command or so?

Philip Nienhuis <philipnienhuis>
Group Member
Mon 24 Feb 2025 11:31:49 PM UTC, comment #6: 

I am confused on what got fixed and where.
This is on Fedora 41, using octave 10:

octave:4> ver
----------------------------------------------------------------------
GNU Octave Version: 10.0.1 (hg id: 8a6da4a3f9e8)
GNU Octave License: GNU General Public License
Operating System: Linux 6.12.15-200.fc41.x86_64 #1 SMP PREEMPT_DYNAMIC Tue Feb 18 15:24:05 UTC 2025 x86_64
----------------------------------------------------------------------
Package Name  | Version | Installation directory
--------------+---------+-----------------------
          io  |   2.6.5 | /home/dmitri/.local/share/octave/api-v59/packages/io-2.6.5


"io" package is from "1474d3b9299a tip"

octave:2> pkg test io
Testing functions in package 'io':

Integrated test scripts:

                                                                                  [ CPU    /  CLOCK ]
  ..packages/io-2.6.5/x86_64-pc-linux-gnu-api-v59/cell2csv.cc-tst  pass    2/2    [ 0.024s /  0.024s]
  ../packages/io-2.6.5/x86_64-pc-linux-gnu-api-v59/col2num.cc-tst  pass   12/12   [ 0.008s /  0.008s]
  ..packages/io-2.6.5/x86_64-pc-linux-gnu-api-v59/csv2cell.cc-tst  pass   18/18   [ 0.045s /  0.045s]
  ../packages/io-2.6.5/x86_64-pc-linux-gnu-api-v59/num2col.cc-tst  pass    3/3    [ 0.002s /  0.002s]
  ..ocal/share/octave/api-v59/packages/io-2.6.5/calccelladdress.m  pass    9/9    [ 0.007s /  0.007s]
  ..itri/.local/share/octave/api-v59/packages/io-2.6.5/fromJSON.m  pass  138/152  [ 0.815s /  0.821s]
                                                                   FAIL   14
  ..i/.local/share/octave/api-v59/packages/io-2.6.5/object2json.m  pass    2/2    [ 0.009s /  0.009s]
  ...local/share/octave/api-v59/packages/io-2.6.5/read_namelist.m  pass    3/3    [ 0.019s /  0.019s]
  ..dmitri/.local/share/octave/api-v59/packages/io-2.6.5/toJSON.m  pass   54/56   [ 0.087s /  0.078s]
                                                                   FAIL    2

Fixed test scripts:

                                                        total time (CPU / CLOCK)  [   1.1s /    1.1s]
Failure Summary:

  ..itri/.local/share/octave/api-v59/packages/io-2.6.5/fromJSON.m  pass  138/152
                                                                   FAIL   14
  ..dmitri/.local/share/octave/api-v59/packages/io-2.6.5/toJSON.m  pass   54/56
                                                                   FAIL    2

Summary:

  PASS                              241
  FAIL                               16


E.g.:

octave:2> test fromJSON verbose
>>>>> /home/dmitri/.local/share/octave/api-v59/packages/io-2.6.5/fromJSON.m
***** test        ##        input validation
 assert( fromJSON(),[]); % ok, reference
***** warning <invalid SARRAY> fromJSON('',struct);

!!!!! warning failed.
Expected <invalid SARRAY>, but got <implicit conversion from string to real N-D array>

octave:3> test toJSON verbose
>>>>> /home/dmitri/.local/share/octave/api-v59/packages/io-2.6.5/toJSON.m
***** test ## invalid args
***** warning <invalid PREC>   toJSON(0,struct);
!!!!! warning failed.
Expected <invalid PREC>, but got <Using an object of size 1x2 as a boolean value implies all().>


Dmitri A. Sergatskov <dasergatskov>
Mon 17 Feb 2025 02:01:32 PM UTC, comment #5: 

In the mean time I fixed toJSON.m to pass all tests again, along the lines Arun showed in comment #15 over in bug #41028.
There's still another similarly doubtful line elsewhere in the file, however that doesn't seem to interfere in the tests.

Fixing fromJSON.m may take a bit more time, overall code quality is not that clear and hard to follow and -most of all- I'm unfamiliar with JSON.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 13 Feb 2025 09:44:02 AM UTC, comment #4: 

@dasergatsov:
Reading back the long thread in patch #9980 and my arguments and musings about including the to/fromJSON functions in the io package, it seems your question isn't quite OT :-)  Future deprecation was already alluded to from the beginning, for when usage of older Octave w/o jsonen-/decode (r. < 7) was expected to be getting sufficiently low.

Yet AFAIR, and reading back e.g., comment #3 in patch #9980, these functions treated JSON constructs sometimes differently than jsonen-/decode in core Octave. I don't know if those differences are still valid (e.g., do jsonen-/decode treat accept numbers?), nor if they're even relevant these days. Reiterating what I wrote in patch #9980, I know next to nothing about JSON.
Core Octave functions should be as much ML-compatible as can be, but I have no objection at all to having similar functions in an add-on pkg that work a little differently. IOW I'm all about offering choice - as long as that's reasonably doable for maintainers.

I tend to try to keep from/toJSON in the io package at least for the next release (hopefully soon), but -indeed- maybe deprecate them or at least advice to not use them ("obsolete" or so).

As to the performance, yes that was already tested in patch #9980. to/fromJSON were assumed to be used for reasonably sized files.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 13 Feb 2025 02:03:10 AM UTC, comment #3: 

Somewhat off-topic -- should "fromJSON" "toJSON" be deprecated in favor of jsondecode/jsonencode? As far as I can tell there is no
code in "io" package uses those functions.

I also tried some JSON files from
https://siko1056.github.io/blog/2020/08/19/gsoc2020-json-benchmark.html


octave:2> pkg load io
octave:3> jstr = fileread("citm_catalog.json");
octave:4> whos jstr
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         jstr        1x1727204              1727204  char

Total is 1727204 elements using 1727204 bytes

octave:5> tic; jsondecode(jstr); toc
Elapsed time is 0.0153322 seconds.
octave:6> tic; fromJSON(jstr); toc
Elapsed time is 41.3967 seconds.
octave:7>


With the "large-file.json"

octave:7> jstr = fileread("large-file.json");
octave:8> whos jstr
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         jstr        1x26141343            26141343  char

Total is 26141343 elements using 26141343 bytes

octave:9> tic; jsondecode(jstr); toc
Elapsed time is 0.211533 seconds.
octave:11> tic; fromJSON(jstr); toc
warning: fromJSON: unclosed array `[{"id":"2489651045","type" ... ":"2015-01-01T15:59:59Z"}
]`

(it goes into swap and eventually got killed by OOM on 64GB machine)

Dmitri.
--


Dmitri A. Sergatskov <dasergatskov>
Wed 12 Feb 2025 09:46:11 PM UTC, comment #2: 

(I already typed a lengthy comment this afternoon but it seems it didn't come through ...)

Hi Markus,

Thanks, and yes I know about this from tests I ran in the course of upgrading the io package during the last weeks.
These functions still worked fine in 8.2.0 and in Octave-10.0.0 built on Jan. 27:

GNU Octave, version 10.0.0
:
:
>> __vm_enable__ (0)   ## enabling the bytecode interpreter makes fromJSON tests go awry
>> test toJSON
PASSES 56 out of 56 tests
>> test fromJSON
PASSES 152 out of 152 tests
>>

... so I think this is due to a recent change in Octave. Indeed, the fix of bug #41028 is a candidate.

First off, the bytecode interpreter is interfering.  Too bad, as I've been using it all the time in my projects where it gives a significant performance boost and I saw no issues yet.

Second, this is contributed code, see patch #9980, where I repeated a statement to the contributor in the help ML:

> I'm not going to maintain them, that's up to you as author and maybe other volunteers. As package maintainer I'm just the first to make sure the package works, its functions have adequate documentation and no severe bugs.

That written, I'll do my best to get these functions working again asap.

In toJSON.m the problems are with the behavior of 'warning' in tests. Running the code and warning code statements separately just works, but in a test setting they don't. IMO this is a new bug in Octave:

>> toJSON(0,struct)
warning: toJSON.m: invalid PREC
ans = 0

>> %!warning <invalid PREC>   toJSON(0,struct)    ## no problems

>> test toJSON
***** warning <invalid PREC>   toJSON(0,struct);
!!!!! warning failed.
Expected <invalid PREC>, but got <Using an object of size 1x2 as a boolean value implies all().>
>>

... and further on it shows that Octave quits completely after a faulty test. Is there no try-catch in the 'pkg test' code?

Commenting out the two warning tests yields:

>> test toJSON
PASSES 54 out of 54 tests


As to fromJSON, the bytecode interpreter interferes, also in older Octave-10.0.0:

>> __vm_enable__ (1)
>> test fromJSON
warning: auto-compilation of fromJSON failed with message VM error 3225: internal VM compiler consistency check failed, m_ignored_ip_start
warning: called from
    __test__ at line 3 column 2
    test at line 682 column 11

warning: auto-compilation of _get_block_ failed with message VM error 3225: internal VM compiler consistency check failed, m_ignored_ip_start
warning: called from
    fromJSON>_fromjson_ at line 114 column 6
    fromJSON at line 107 column 3
    __test__ at line 3 column 2
    test at line 682 column 11

***** warning <invalid SARRAY> fromJSON('',struct);

!!!!! warning failed.
Expected <invalid SARRAY>, but got <auto-compilation of _get_block_ failed with message VM error 3225:
 internal VM compiler consistency check failed, m_ignored_ip_start>

I have little time now, but I'll try to sort out a reproducible test piece for a bytecode interpreter bug  report later on.

As far as this bug goes, I'll link to bug #41028.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 12 Feb 2025 04:25:55 PM UTC, comment #1: 

Maybe, related to the changes in bug #41028.

Markus Mützel <mmuetzel>
Group administrator
Wed 12 Feb 2025 09:35:49 AM UTC, original submission:  

The CI is up and running again after the merge of the default branch to the stable branch in preparation of Octave 10.

With that, I see the following failing tests for the io package:
https://github.com/gnu-octave/octave-buildbot/actions/runs/13273893349/job/37059415356#step:14:6448

  Failure Summary:

    ..1-02-23-w64\mingw64\share\octave\packages\io-2.6.4\fromJSON.m  pass  138/152
                                                                     FAIL   14
    ..-11-02-23-w64\mingw64\share\octave\packages\io-2.6.4\toJSON.m  pass   54/56
                                                                     FAIL    2


Verbose output for these tests:

  >>>>> processing D:\a\octave-buildbot\octave-buildbot\octave-2025-02-11-02-23-w64\mingw64\share\octave\packages\io-2.6.4\fromJSON.m
  ***** warning <invalid SARRAY> fromJSON('',struct);

  !!!!! warning failed.
  Expected <invalid SARRAY>, but got <implicit conversion from string to real N-D array>

  ***** warning <unclosed>    assert(fromJSON('{a:3,b:4'),struct("a",3,"b",4));
  !!!!! warning failed.
  Expected <unclosed>, but got <Using an object of size 2x1 as a boolean value implies all().>

  ***** warning <malformed> assert( fromJSON("{'a  :3}"), struct());
  !!!!! warning failed.
  Expected <malformed>, but got <Using an object of size 1x61 as a boolean value implies all().>

  ***** warning <malformed> assert( fromJSON("{'a\":3}"), struct());
  !!!!! warning failed.
  Expected <malformed>, but got <Using an object of size 1x61 as a boolean value implies all().>

  ***** warning <malformed> assert( fromJSON("{\"a :3}"), struct());
  !!!!! warning failed.
  Expected <malformed>, but got <Using an object of size 1x61 as a boolean value implies all().>

  ***** warning <malformed> assert( fromJSON("{\"a':3}"), struct());
  !!!!! warning failed.
  Expected <malformed>, but got <Using an object of size 1x61 as a boolean value implies all().>

  ***** warning <malformed> assert( fromJSON('{a:3,b   }', true),
                              struct('a',3));
  !!!!! warning failed.
  Expected <malformed>, but got <Using an object of size 1x61 as a boolean value implies all().>

  ***** warning <malformed> assert( fromJSON('{a:3,b:   }', true),
                              struct('a',3));
  !!!!! warning failed.
  Expected <malformed>, but got <Using an object of size 1x61 as a boolean value implies all().>

  ***** warning <invalid> assert( fromJSON('[😊,4]'), [NaN, 4]); % convert to NaN
  !!!!! warning failed.
  Expected <invalid>, but got <Using an object of size 1x2 as a boolean value implies all().>

  ***** warning <invalid> assert( fromJSON("[1, gar bage]"),[1,NaN])
  !!!!! warning failed.
  Expected <invalid>, but got <Using an object of size 1x2 as a boolean value implies all().>

  ***** warning <invalid> assert( fromJSON("[1, \n gar \n bage]"),[1,NaN])
  !!!!! warning failed.
  Expected <invalid>, but got <Using an object of size 1x2 as a boolean value implies all().>

  ***** warning <invalid> assert( fromJSON('[1,gar""bage]'),[1,NaN])
  !!!!! warning failed.
  Expected <invalid>, but got <Using an object of size 1x2 as a boolean value implies all().>

  ***** warning <invalid> assert( fromJSON('[1,gar[]bage]'),[1,NaN])
  !!!!! warning failed.
  Expected <invalid>, but got <Using an object of size 1x2 as a boolean value implies all().>

  ***** warning <invalid> assert( fromJSON('[1,{"a":gar{}bage}]'),{1,struct('a',NaN)})
  !!!!! warning failed.
  Expected <invalid>, but got <Using an object of size 1x2 as a boolean value implies all().>

[...]

  >>>>> processing D:\a\octave-buildbot\octave-buildbot\octave-2025-02-11-02-23-w64\mingw64\share\octave\packages\io-2.6.4\toJSON.m
  ***** warning <invalid PREC>   toJSON(0,struct);
  !!!!! warning failed.
  Expected <invalid PREC>, but got <Using an object of size 1x2 as a boolean value implies all().>

  ***** warning <invalid COMPACT> toJSON(0,:,struct);
  !!!!! warning failed.
  Expected <invalid COMPACT>, but got <Using an object of size 1x2 as a boolean value implies all().>


It looks like different warnings (or different wording?) are triggered with Octave 10 compared to previous versions.

CC'ing Philip as the package maintainer.

Markus Mützel <mmuetzel>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #56952:  _bfilter_.m.gz added by dasergatskov (673B - application/x-gzip)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by mmuetzel (Submitted the item)
  • -email is unavailable- added by mmuetzel
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2025-02-25 dasergatskov Attached File- Added bfilter.m.gz, #56952
    2025-02-25 philipnienhuis Summary[octave-forge] (io) Failing tests with Octave 10 [octave-forge] (io) Failing tests with Octave 10 in toJSON.m and fromJSON.m
    2025-02-17 philipnienhuis StatusNone In Progress
        Assigned toNone philipnienhuis
    2025-02-12 philipnienhuis Dependencies- Depends on bugs #41028
    2025-02-12 mmuetzel Carbon-Copy- Added philipnienhuis

    Back to the top

    Powered by Savane 3.14-7003.
    Corresponding source code