Creating C and Python bindings for all functionality used by Repl. Refactoring Repl to call through Python by default #450

luketpeterson · 2023-10-02T07:34:57Z

Creating C and Python bindings for all functionality used by Repl.
Refactoring Repl to call through Python by default.

Changes include:

Adding Environment object to Rust, C & Python for non-interactive configuration management (module search paths, config files, etc.)
Adding SyntaxNode parsing API to C & Python
Adding RunnerState / incremental MeTTa runner API to C & Python
Adding parse error accessor to C & Python, instead of panicking on invalid syntax
Refactoring REPL app, so that it calls MeTTa through Python by default. NOTE: I am preserving the direct Rust path for the "no_python" path because I plan to unify the "python" and "no_python" paths in the future, once we can link HyperonPy in a static build. (when we address PyO3 as alternative Python binding #283)
Changing REPL default to Python, and requiring explicit "no_python" feature to build REPL without Python dependency
Removing ctor to initialize logger, because it makes sense to init the logger from the environment initialization

…n-UI parts of the REPL environment

…action, in the Python build path

…to initialize the logger (the platform Environment) See trueagi-io#314

luketpeterson · 2023-10-02T07:40:41Z

This PR should address:
#439
#386
#301

And perhaps others.

vsbogd

Luke, thanks for the such amount of work. Didn't look at the REPL code changes yet, but posting current comments to not slow this down.

lib/src/metta/environment.rs

python/hyperon/base.py

python/hyperon/atoms.py

python/tests/scripts/f1_imports.metta

lib/src/metta/runner/mod.rs

lib/src/lib.rs

python/hyperon/runner.py

c/src/metta.rs

…default initialization was used

…ly fail Reworking C API to use builder object, rather than relying on a static state. This allows flexibility to initialize the environment from multiple threads - although that is still ugly and discouraged

…vironments to be initialized and used from C & Python layers

vsbogd

Luke, I am very sorry for a late review. Thanks for adding ability to initialize runner with instance of the environment 👍 I propose to go further with fixing the rest of comments and merging it.

One major question to me here is should we keep some shared instance of the environment or just replace it by asking user to create a default instance if it is needed?

lib/src/metta/environment.rs

vsbogd · 2023-10-10T10:08:08Z

lib/src/metta/runner/mod.rs

+    ///
+    /// NOTE: If `env_builder` is `None`, the platform environment will be used
+    /// NOTE: This function does not load any stdlib atoms, nor run the [Environment]'s 'init.metta'
+    pub fn new_with_space(space: DynSpace, tokenizer: Shared<Tokenizer>, env_builder: Option<EnvBuilder>) -> Self {


Did you intentionally use EnvBuilder instead of Environment here? We could also pass a Builder a Environment instance here.

Yes. I felt like having the built environment in an owned state complicated the API (especially in other languages) without much gain.

So now Environment is like Path; ie. the API user can have access &Environment references, and call methods on it, but they can't own a naked Environment outside the hyperon crate.

python/hyperon/runner.py

lib/src/metta/runner/mod.rs

…vec_t with a Python list of atoms Changing `sexpr_parser_t` from wrapped Shared ptr to wrapped Box Changing RunnerState API to take ownership of the parser and, rather than asking the caller to provide the parser each step. There is no good reason to switch parsers with the same RunnerState and switching them is just asking for bugs Adding “with_atoms” alternative to create a RunnerState

…ific stdlib loader

…se.py, because error expressions are not part of the core MeTTa spec

…APIs, and adding some infrastructure around error expression atoms

luketpeterson · 2023-10-13T01:01:03Z

I believe I have addressed all of the feedback in this PR.

The only remaining issue is whether or not runner.load_py_module_from_path should be private, and any new issues resulting from the changes I made.

lib/src/metta/runner/mod.rs

vsbogd · 2023-10-16T13:44:37Z

lib/src/metta/mod.rs

+        None => EMPTY_SYMBOL,
+    };
+    if let Some(err_code) = err_code {
+        Atom::expr([ERROR_SYMBOL, err_atom, err_code, Atom::sym(message)])


At the moment Error expressions use only two arguments: first is atom and second one is either error code or custom message. Also atom is required argument. Which use-cases do you have in mind for the changes made?

The idea was that sometimes both an err_code and a custom message are desirable. The caller can branch on an err code in a way that they can't reliably branch on a human-readable message. But a human-readable message can specify additional information, including composing a message with specific details about the error.

vsbogd · 2023-10-16T13:52:21Z

lib/src/metta/mod.rs

+
+//QUESTION: These functions below seem to only be used in tests.  If that's the case, should we
+// move them into a test-only module so they aren't exposed as part of the public API?
+//Alternatively rewrite the tests to use a runner?


Yes please move them into a test-only module for now. There is no much sense in re-writing these tests, they will vanish after migration to the minimal interpreter.

c/src/metta.rs

python/tests/test_extend.py

…r with a custom space

…er handles Re-adding runner-equivalence test

… doesn't appear to be the same as encountering parse errors

vsbogd

Thanks Luke, I approved it, please feel free to merge when you are ready.

luketpeterson · 2023-10-18T16:19:50Z

I think this is ready for merge after these fixes. As discussed, I'll take care of the extraneous API in the metta/mod.rs file in a separate PR.

vsbogd · 2023-10-20T08:53:38Z

Thanks Luke, I merged it

Necr0x0Der · 2023-10-20T09:45:35Z

Thanks Luke. Looks cool and working!

luketpeterson added 13 commits September 23, 2023 16:32

Initial draft of API-wide MeTTa Environment object, to replace the no…

1a395b7

…n-UI parts of the REPL environment

Transitioning Environment config API to a "builder" style API

00d62fc

Updating C & Python runner init code to use Environment by default

c05e12e

Adding C API for platform environment config

95cd2ce

Exposing environment configuration as a Python API

24a0d63

Oops forgot to git add file

6d9d06e

Exposing C Api for parsing into syntax nodes

adee13e

Adding Python interface to SyntaxNode parsing

fd3b6c1

Adding C interface to incremental runner

9906182

Exposing incremental runner in Python API

1de9d45

Adding pathway to access parse errors in C & Python APIs

a181df9

Refactoring the repl app to call through python for every MeTTa inter…

6679de9

…action, in the Python build path

Getting rid of ctor, since we now have a top-level object from which …

9c6dd2c

…to initialize the logger (the platform Environment) See trueagi-io#314

luketpeterson requested a review from vsbogd October 2, 2023 07:35

Merge branch 'main' into repl

b504aeb

vsbogd reviewed Oct 2, 2023

View reviewed changes

luketpeterson and others added 6 commits October 3, 2023 14:52

Fixing bug where logger wasn't initialized when platform environment …

b80a23f

…default initialization was used

Adding try_init so platform environment initialization can graceful…

586e2b2

…ly fail Reworking C API to use builder object, rather than relying on a static state. This allows flexibility to initialize the environment from multiple threads - although that is still ugly and discouraged

Merge branch 'main' into repl

3531f6f

Merge branch 'main' into repl

286c696

Extensive rework of Environment API, to permit multiple coexisting en…

dc15fc3

…vironments to be initialized and used from C & Python layers

Merge branch 'main' into repl

19df80e

vsbogd reviewed Oct 11, 2023

View reviewed changes

luketpeterson and others added 6 commits October 11, 2023 13:06

Merge branch 'main' into repl

2082e52

Refactoring runner into process with a hook to insert a language-spec…

3ac2582

…ific stdlib loader

Converting Atom.is_error() in atoms.py into atom_is_error() in ba…

205b4d0

…se.py, because error expressions are not part of the core MeTTa spec

Moving Environment sub-module into runner

3be64f5

Renaming "platform" environment to "common" environment

9029f66

Updating the way parse errors are returned from the C & Python parse …

a3808f1

…APIs, and adding some infrastructure around error expression atoms

vsbogd reviewed Oct 16, 2023

View reviewed changes

luketpeterson added 3 commits October 16, 2023 18:01

Adding back the ability to create a fully-initialized top-level runne…

790ad1e

…r with a custom space

Adding runner_eq() function to compare the runners referenced by runn…

8254d3f

…er handles Re-adding runner-equivalence test

Re-Adding side-band error accessor to parser, so parsing parse errors…

d5253d7

… doesn't appear to be the same as encountering parse errors

vsbogd previously approved these changes Oct 18, 2023

View reviewed changes

Fixing benchmarks and tests to compile with all the API changes

d649ba1

luketpeterson dismissed vsbogd’s stale review via d649ba1 October 18, 2023 16:18

luketpeterson and others added 3 commits October 18, 2023 09:47

one last fix so repl's "no_python" feature uses new API

fe6c15d

Merge branch 'main' into repl

f5cef9a

GitHub.com editor messed up indentation in last merge. Fixing.

f31df56

luketpeterson mentioned this pull request Oct 19, 2023

Rework rust test functions & add stdlib comparison operators #462

Merged

vsbogd approved these changes Oct 20, 2023

View reviewed changes

vsbogd merged commit 738e739 into trueagi-io:main Oct 20, 2023
1 check passed

luketpeterson mentioned this pull request Oct 20, 2023

Need accessor for loaded module's space, within MeTTa #466

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Creating C and Python bindings for all functionality used by Repl. Refactoring Repl to call through Python by default #450

Creating C and Python bindings for all functionality used by Repl. Refactoring Repl to call through Python by default #450

luketpeterson commented Oct 2, 2023 •

edited

Loading

luketpeterson commented Oct 2, 2023

vsbogd left a comment

vsbogd left a comment

vsbogd Oct 10, 2023

luketpeterson Oct 12, 2023

luketpeterson commented Oct 13, 2023

vsbogd Oct 16, 2023

luketpeterson Oct 17, 2023

vsbogd Oct 16, 2023

vsbogd left a comment

luketpeterson commented Oct 18, 2023

vsbogd commented Oct 20, 2023

Necr0x0Der commented Oct 20, 2023

Creating C and Python bindings for all functionality used by Repl. Refactoring Repl to call through Python by default #450

Creating C and Python bindings for all functionality used by Repl. Refactoring Repl to call through Python by default #450

Conversation

luketpeterson commented Oct 2, 2023 • edited Loading

luketpeterson commented Oct 2, 2023

vsbogd left a comment

Choose a reason for hiding this comment

vsbogd left a comment

Choose a reason for hiding this comment

vsbogd Oct 10, 2023

Choose a reason for hiding this comment

luketpeterson Oct 12, 2023

Choose a reason for hiding this comment

luketpeterson commented Oct 13, 2023

vsbogd Oct 16, 2023

Choose a reason for hiding this comment

luketpeterson Oct 17, 2023

Choose a reason for hiding this comment

vsbogd Oct 16, 2023

Choose a reason for hiding this comment

vsbogd left a comment

Choose a reason for hiding this comment

luketpeterson commented Oct 18, 2023

vsbogd commented Oct 20, 2023

Necr0x0Der commented Oct 20, 2023

luketpeterson commented Oct 2, 2023 •

edited

Loading