run_step_by_step in runner

[noskill]

I find running step by step especially useful for studying a new language, even for declarative languages

[luketpeterson]

The API usage is correct, but I am unclear on who you envision as the client for run_step_by_step.

What I mean is that it is a lower-level interface than MeTTa.run but a higher-level interface than creating a RunnerState manually. Personally, I don't know if the solution space has enough surface area to require 3 different interfaces. Perhaps we could eliminate the RunnerState object from the Python API in favor of a yielding MeTTa method like you implemented.

If we do decide to go this route, I'd want to make two changes:

1. I think the flat argument should work the same way it does for MeTTa.run
2. current_results provides access to the whole results list, but the yielding design feels like it should provide only the new results each step. So IMO the easy fix would be to add de-duplication logic to Python, but a better fix might be to plumb the incremental results accessor all the way from the Rust core.

Lastly, I think there should be a test. I quickly wrote this (based on the example from Patrick Hammer), but perhaps you have an idea for a test with better coverage.

    def test_run_step_by_step(self):
        program = '''
            (= (TupleCount $tuple) (if (== $tuple ()) 0 (+ 1 (TupleCount (cdr-atom $tuple)))))
            (= tup (1 2))
            (= tup (3 4 5))
            !(match &self (= tup $t) (TupleCount $t))
        '''
        runner = MeTTa(env_builder=Environment.test_env())
        for _state, results in runner.run_step_by_step(program):
            pass
        self.assertEqual(len(results[0]), 2)

[Necr0x0Der]

Personally, I don't know if the solution space has enough surface area to require 3 different interfaces.

I wouldn't like to eliminate the lower-level RunnerState API in Python. run_step_by_step is convenient for analysing behavior. Since @noskill has introduced it for covering his needs, than it makes sense to have both. I also have some doubts regarding the number of parse_* and load_py_module_* functions in runner.py . And previously, there was a moment of refactoring of numerous parsing and running functions in the runner. But there is still the lack of some useful interface functions like running an expression in a space without parsing a string. My personal opinion is that it is more convenient to first introduce all the functions requested for particular needs, and then systematise the API in a case-driven fashion rather than restricting the API beforehand.

[noskill]

@luketpeterson
I want to access intermediate steps from python to display execution trace like in swi-prolog, though i will have to make additional PRs since there are too many RunnerState and i have hard time finding important steps of interest e.g. backtracking

memb(X, [X| L]).
memb(X, [_| L]) :-
    memb(X, L).

trace:

[trace] 1 ?- memb(A, [1,2]), memb(A, [2,3]).
   Call: (11) memb(_5772, [1, 2]) ? creep
   Exit: (11) memb(1, [1, 2]) ? creep
   Call: (11) memb(1, [2, 3]) ? creep
   Call: (12) memb(1, [3]) ? creep
   Call: (13) memb(1, []) ? creep
   Fail: (13) memb(1, []) ? creep
   Fail: (12) memb(1, [3]) ? creep
   Fail: (11) memb(1, [2, 3]) ? creep
   Redo: (11) memb(_5772, [1, 2]) ? creep
   Call: (12) memb(_5772, [2]) ? creep
   Exit: (12) memb(2, [2]) ? creep
   Exit: (11) memb(2, [1, 2]) ? creep
   Call: (11) memb(2, [2, 3]) ? creep
   Exit: (11) memb(2, [2, 3]) ? creep
A = 2 ;
   Redo: (11) memb(2, [2, 3]) ? creep
   Call: (12) memb(2, [3]) ? creep
   Call: (13) memb(2, []) ? creep
   Fail: (13) memb(2, []) ? creep
   Fail: (12) memb(2, [3]) ? creep
   Fail: (11) memb(2, [2, 3]) ? creep
   Redo: (12) memb(_5772, [2]) ? creep
   Call: (13) memb(_5772, []) ? creep
   Fail: (13) memb(_5772, []) ? creep
   Fail: (12) memb(_5772, [2]) ? creep
   Fail: (11) memb(_5772, [1, 2]) ? creep
false.

It's not perfect representation, but it easier to read than RunnerState, for example

Call: (11) memb(_5772, [1, 2]) ? creep
Exit: (11) memb(1, [1, 2]) ? creep

call here is the same as in metta, it will try to unify variable, Exit means successful unification _5772=1
then the inference engine tries memb(A=1, [2,3]) and fails
"Redo: (11) memb(_5772, [1, 2]) ? creep" is backtracking to previously tried variant A=1

The same program in metta

        (= (memb $X Nil) False)
        (= (memb $X (Cons $H $Tail))
           (memb $X $Tail))
        (= (memb $X (Cons $X $Tail))
           True)
        !(let $res (and (memb $X (Cons 1 (Cons 2 Nil))) (memb $X (Cons 2 (Cons 3 Nil))))   (if $res $X None))

also @vsbogd

[luketpeterson]

Thanks for explaining that, @noskill. It sounds like some better accessors for RunnerState's internals will go a long way to provide the debug view you are looking for.

[luketpeterson]

My issue with a yielding API is that each step yields the complete set of results, as opposed to the incremental results. So collecting the results into a list will potentially result in many duplicate values in the list. Does my concern make sense, or am I misunderstanding how you intend the API to be used?

I totally see the value in better debug information from RunnerState. I filed this #492 to track that separately.

Finally, does my comment about the unnecessary code (below) make sense? Or did I misunderstand something?

[luketpeterson]

state.is_complete() starts out returning False, so I don't see the need to execute these 3 lines before entering the loop.

This function should behave identically unless program contains no parsable atoms:

    def run_step_by_step(self, program):
        """Runs program step by step, yielding state and result"""
        state = RunnerState(self, program)
        while not state.is_complete():
            state.run_step()
            results = state.current_results()
            yield state, results

[luketpeterson]

Sorry I didn't realize I didn't submit this last week.

[luketpeterson]

@noskill I would like to hold off merging this because some major changes to the RunnerState object are underway. We may end up eliminating the RunnerState object altogether, and replace it with a delegate object that can be used to monitor for updates, interrupt the execution, and get debug information.

It looks like the changes will bring the underlying Rust API closer to what you are doing with the yielding Python method. But it's a heavy WIP.

[vsbogd]

@noskill , I am not sure whether we need to resolve conflicts and merge or cancel it?

[luketpeterson]

@noskill , I am not sure whether we need to resolve conflicts and merge or cancel it?

Actually it was me who is holding up this merge. I felt like the runner's API needed some changes to support multi-threading, and this PR got us further from where we need to be.

The reason I didn't close it is because I wanted to keep it as a place to track the suggestions for the Python API, and because I didn't have a replacement to offer yet.