Kappa-Dev / KappaTools

Tool suite for kappa models. Documentation and binaries can be found in the release section. Try it online at
http://kappalanguage.org/
GNU Lesser General Public License v3.0
112 stars 41 forks source link

Using the $SNAPSHOT perturbation corrupts generated trace files? #656

Closed jonathan-laurent closed 1 year ago

jonathan-laurent commented 1 year ago

An example model that @hmedina shared with me produces corrupted trace files. Here is the model:

%agent: A(b, x{i, j})
%agent: B(a, c)
%agent: C(b, x{k, q})

%init: 1 A(b[1], x{i}), B(a[1], c[2]), C(b[2])
%init: 1 A(b[1], x{j}), B(a[1], c[2]), C(b[2])

B(a[_])- @ 10
A(x{i}, b[_/2]), A(x{j}, b[1/.]), B(a[1/2]) @ 1

C(x{k}) <-> C(x{q}) @ 10, 10

%mod: |B()| = 0 do $SNAPSHOT "snap".[E].".json"; $SNAPSHOT [true] ; $STOP ;  // BUG
%mod: |B()| = 0 do $STOP ;  // OK

When running KaSim on this model, generating a JSON trace and calling Trace.get_headers_from_file from this trace, the following exception is raised:

Fatal error: exception Yojson.Basic.Util.Type_error("Invalid modification", _)
Raised at Kappa_terms__Primitives.modification_of_yojson in file "core/term/primitives.ml", line 597, characters 9-83
Called from Stdlib__List.rev_map.rmap_f in file "list.ml", line 103, characters 22-25
Called from Kappa_generic_toolset__JsonUtil.to_list in file "core/dataStructures/jsonUtil.ml", line 87, characters 10-43
Called from Kappa_terms__Primitives.perturbation_of_yojson in file "core/term/primitives.ml", line 639, characters 13-104
Called from Kappa_generic_toolset__Tools.array_map_of_list.(fun) in file "core/dataStructures/tools.ml", line 140, characters 31-36
Called from Kappa_terms__Model.of_yojson in file "core/term/model.ml", line 370, characters 12-149
Called from Kappa_runtime__Trace.get_headers_from_file in file "core/simulation/trace.ml", line 476, characters 12-102

Thus, the part that breaks is the deserialization of the model (no trace step is being accessed here). In this case, I noticed that removing the $SNAPSHOT perturbation solves the issue, which is also hinted by the stack trace.

There is probably an issue in the JSON serialization/deserialization code to be fixed here. More generally, this adds to the case for autogenerating serialization code in the future.

hmedina commented 1 year ago

The following model also produces "corrupted" traces. The model is just one line.

%mod: [E] = 0 do $SNAPSHOT ;
hmedina commented 1 year ago

It seems the issue is a missing case in https://github.com/Kappa-Dev/KappaTools/blob/777835b82f449d3d379713df76ff25fd5926b762/core/term/primitives.ml#L501

Unless I'm misreading the syntax, it seems to me none of the 10 cases for snapshots (lines 551-563) allow an elided file name.

Whereas this one-liner model yields a "corrupted" trace[^1] :

%mod: [E] = 0 do $SNAPSHOT ;
 "effect": [
                    {
                        "action": "SNAPSHOT",
                        "raw": false
                    }
                ],

This one yields a "valid" trace[^1] :

%mod: [E] = 0 do $SNAPSHOT "snap.ka" ;
 "effect": [
                    {
                        "action": "SNAPSHOT",
                        "raw": false,
                        "file": [
                            {
                                "val": "snap.ka",
                                "loc": {
                                    "file": 1,
                                    "bline": 2,
                                    "bchr": 27,
                                    "echr": 36
                                }
                            }
                        ]
                    }
                ],

[^1]: Trace snippets; these are the JSON blocks in /model/interventions/effect

feret commented 1 year ago

The pattern matching for dealing with SNAPSHOT event was incomplete. Since JsonUtil.smart_assoc is used, optional fields are removed. 2 cases were missing in the pattern matching in primitives.ml to handle this.

feret commented 1 year ago

@jonathan-laurent, @hmedina Could you check whether your problem is solved ?

hmedina commented 1 year ago

Using the model at the head of this thread, and a query, the parsing worked as expected.

first query ``` query 'foo.csv' match e: { -B: B() } return {'t': time[e]} ```

To stress test, I used the following model to test every way I know to invoke the snapshot operation, and the resulting trace was properly parsed by KaTIE's invocation of the KaTools.

%agent: timer(s{tik, tok})      %init: 1 timer()
timer(s{tik}) <-> timer(s{tok}) @ 1, 1

%mod: [E] = 1 do $SNAPSHOT ;
%mod: [E] = 2 do $SNAPSHOT [true] ;
%mod: [E] = 3 do $SNAPSHOT [false] ;
%mod: [E] = 4 do $SNAPSHOT [true] ; repeat [true]
%mod: [E] = 5 do $SNAPSHOT [true] ; repeat [false]
%mod: [E] = 6 do $SNAPSHOT [false] ; repeat [true]
%mod: [E] = 7 do $SNAPSHOT [false] ; repeat [false]

%mod: [E] = 11 do $SNAPSHOT "snap_".[E].".ka" ;
%mod: [E] = 12 do $SNAPSHOT "snap_".[E].".ka" [true]  ;
%mod: [E] = 13 do $SNAPSHOT "snap_".[E].".ka" [false] ;
%mod: [E] = 14 do $SNAPSHOT "snap_".[E].".ka" [true]  ; repeat [true]
%mod: [E] = 15 do $SNAPSHOT "snap_".[E].".ka" [true]  ; repeat [false]
%mod: [E] = 16 do $SNAPSHOT "snap_".[E].".ka" [false] ; repeat [true]
%mod: [E] = 17 do $SNAPSHOT "snap_".[E].".ka" [false] ; repeat [false]

%mod: [E] = 21 do $SNAPSHOT "snap_".[E].".json" ;
%mod: [E] = 22 do $SNAPSHOT "snap_".[E].".json" [true]  ;
%mod: [E] = 23 do $SNAPSHOT "snap_".[E].".json" [false] ;
%mod: [E] = 24 do $SNAPSHOT "snap_".[E].".json" [true]  ; repeat [true]
%mod: [E] = 25 do $SNAPSHOT "snap_".[E].".json" [true]  ; repeat [false]
%mod: [E] = 26 do $SNAPSHOT "snap_".[E].".json" [false] ; repeat [true]
%mod: [E] = 27 do $SNAPSHOT "snap_".[E].".json" [false] ; repeat [false]

%mod: [E] = 30 do $STOP ;
second query ``` query 'tik_to_tok.csv' match tiktok: { timer(s{tik/tok}) } return {'t': time[tiktok]} query 'tok_to_tik.csv' match toktik: { timer(s{tok/tik}) } return {'t': time[toktik]} ```

This should perhaps be an integration test for the KaTools, testing the trace it produces can be read by itself? From a quick look through integration tests, it doesn't seem the perturbation directives are tested at all.

feret commented 1 year ago

@jonathan-laurent Sure, please add all the test case that you want in the integration tests. Just make sure to sort them by topic.

jonathan-laurent commented 1 year ago

I added Hector's test to the integration test suite. I used KaStor to load the resulting trace since KaTie isn't integrated into the Kappa distribution yet.

I also added a README.md for the integration test suite. @feret What is your workflow for updating the reference output files when a nontrivial diff is detected but the new version is correct? Using patch manually is painful. Don't you have a command similar to dune promote?