trueagi-io / hyperon-experimental

MeTTa programming language implementation
https://metta-lang.dev
MIT License
147 stars 49 forks source link

A "Context" object could cut down on the number of Rc<RefCell>s needed in Hyperon #410

Open luketpeterson opened 1 year ago

luketpeterson commented 1 year ago

Hyperon's ad-hoc sharing of state using the Rc<RefCell> pattern (or Shared) mainly appears to be in service of a missing "Context" structure (or structures).

This leads to the double-borrow issues caused by recursion, such as https://github.com/trueagi-io/hyperon-experimental/issues/397 or https://github.com/trueagi-io/hyperon-experimental/issues/241

Also, it will make it very difficult to get multi-threaded execution into Hyperon.

I believe the first step is to implement a "Context" struct (maybe separate MatchContext and ExecutionContext) that provide access to the parameters that are currently wrapped in Shared pointers and held by each atom.

We also need to think through how Space updates work (the foundation of a Space monad) to be sure we never get a conflict trying to write to a space while we're simultaneously reading from it.

This is a fairly involved refactor, so probably should happen after Alpha. (I estimate 2 week for the Atom execution and matching environment and maybe another 2 weeks to design and implement the Space monad)

vsbogd commented 1 year ago

"Environment" struct (maybe separate MatchEnvironment and ExecutionEnvironment) that provide access to the parameters that are currently wrapped in Shared pointers and held by each atom.

Luke, could you please elaborate your idea further. Do you suggest replacing smart pointers by handles which will be resolved by ...Environment structures? Are instances of ...Environment structure singletons? Main question is how access to the object will work in multi-threaded environment?

luketpeterson commented 1 year ago

Luke, could you please elaborate your idea further.

Rather than each Grounded atom containing a reference to the shared system resources it needs, through a runtime-protected Shared<T> pointer, those resources would be made available through an argument passed by the caller to the execute or match functions.

So for example, take the AssertEqualOp which is currently implemented as

pub struct AssertEqualOp {
    space: DynSpace,
}

instead of needing to store space inside the atom, space could be a member of an environment, provided when the atom is executed. Like:

    fn execute(&self, env: &ExecEnvironment, args: &[Atom]) -> Result<Vec<Atom>, ExecError> {

This will require a pretty big rework of the Rust interfaces (but at least Rust will tell us if we've done anything illegal so it's not scary). I think we'll come out the other side with a better handle on the internal state management and we'll be able to move forward with things like multi-threading / concurrency.

vsbogd commented 1 year ago

Ok, I got it wrong first. Approach you suggesting makes sense totally. I was trying to implement it in minimal MeTTa where I wanted to implement as much grounded functions as possible in MeTTa itself. My plan was to pass space which is needed to match atoms as an argument to MeTTa functions.

One relatively small technical problem with it is that atoms like assert... are widely used in MeTTa scripts and adding an additional argument means breaking existing code. We also cannot use &self in MeTTa implementation of assert.. because inside MeTTa stdlib it references to MeTTa stdlib atomspace while current working atomspace of the interpreter is expected. Thus my current plan is either keep it as is and merge minimal MeTTa before fixing this or to introduce another predefined MeTTa atom (it could have name &env) which references the current atomspace which is used by interpreter.

luketpeterson commented 1 year ago

I'm going to make a proposal (PR) shortly to add a top-level object that will be called "Environment", essentially to encapsulate all the non-interactive functionality of the REPL. That "Environment" is not what this issue is about, so I'll henceforth call this issue's proposed structure(s) "Context".

vsbogd commented 8 months ago

@luketpeterson is it considered to be completed or modules work is also part of this?