Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

case cannot be used with function calls defined out of the case metta module scope #354

Open
vsbogd opened this issue Jun 30, 2023 · 14 comments
Labels
blocker bug Something isn't working

Comments

@vsbogd
Copy link
Collaborator

vsbogd commented Jun 30, 2023

When case calls the passed function in the imported module, for example (synthesize.metta):

;; Define synthesize
(: synthesize (-> Atom (-> Atom) Atom))
(= (synthesize $query $axiom) (case ($axiom) (($query $query))))

and it is called from importing module, where function is passed as an argument (bug.metta):

;; Import synthesize
!(import! &self synthesize.metta)

;; Define axiom
(: axiom (-> Atom))
(= (axiom) (: f (-> Number String)))

;; Test synthesize
!(synthesize (: $term $type) axiom)

case cannot call the passed function because this function is defined in the outer module while case runs interpreter with space of the module where the call of the case is defined.

Expected result: [(: f (-> Number String))]
Actual result: []

@vsbogd vsbogd added the bug Something isn't working label Jun 30, 2023
ngeiswei added a commit to ngeiswei/hyperon-pln that referenced this issue Jul 18, 2023
unify* does not work yet due to issues
trueagi-io/hyperon-experimental#242 and
trueagi-io/hyperon-experimental#354

It should be noted that even the let/let* variant is bound to
eventually fail due to issue 242 (because sooner or later a pattern
may unify with a function definition in the current atomspace), but it
buys use some time because at least let* cannot be unified with
anything due to being hard coded at the rust level.
@vsbogd
Copy link
Collaborator Author

vsbogd commented Jul 25, 2023

As the space into which pure function is passed as an atom doesn't import the space where the function is defined we cannot execute pure function in it properly. In order to execute the passed pure function we need to have a reference to the space where the function is defined.

Usually this issue is not visible because top-level interpreter works on top-level space which imports all other spaces and thus search of the atom in top-level space brings all needed results. In case from the issue description the search starts from the top level space from which case was imported i.e. from synthesize.metta's space. This space doesn't contain axiom's definition and thus search fails.

@luketpeterson
Copy link
Contributor

The current behavior is essentially correct for import because modules are expected to be self-contained. It would be impossible to validate a module if its environment could be affected implcitly by items from the client.

This situation is where an include behavior is desirable, to introduce additional code into the same running context.

@luketpeterson luketpeterson changed the title case cannot be used with function calls defined out of the case metta module scope include operation needed to access external functionality within the metta module's scope Mar 28, 2024
@vsbogd
Copy link
Collaborator Author

vsbogd commented Mar 28, 2024

This situation is where an include behavior is desirable, to introduce additional code into the same running context.

I am not sure if you mean this but include doesn't help in most of the cases. Consider the situation when module a passes its function into a module b and module b forwards it to the module c. Then module a cannot know in advance it should include something to use module b. Thus we rather need some referencing from atom to module where it is defined.

@luketpeterson
Copy link
Contributor

luketpeterson commented Mar 28, 2024

I don't follow what you are saying... The term "pass into" is confusing me because modules don't explicitly declare any export directives. So any item used must be imported or included by the module that intends to use it.

If we have something like this:
A.metta

(= (synthesize $query $axiom) (case ($axiom) (($query $query))))

B.metta

!(include A)

C.metta

!(include B)
(= (axiom) (: f (-> Number String)))
!(synthesize (: $term $type) axiom)

Then you would get the expected (correct) result.

@vsbogd
Copy link
Collaborator Author

vsbogd commented Mar 28, 2024

Its vice versa. I mean something like this (the example is highly unnatural):
A.metta:

!(import B.metta)
(= (foo $arg) OK)
!(call-b foo)

B.metta:

!(import C.metta)
(= (call-b $callback) (call-c $callback))

C.metta:

(= (call-c $callback) ($callback some-arg))

Actually it doesn't even matter whether is chain consists of three or two modules. It indeed works if we replace import by include. But If only way to get higher order library function is "including" module where it is defined then include will be used everywhere instead of import which is not effective.

@vsbogd
Copy link
Collaborator Author

vsbogd commented Mar 28, 2024

I have very similar issue with get-docs (here #640). It should get docs from the module where atom passed as argument is defined but there is no way of doing this. &self points to the space where get-docs is defined. In addition get-type after recent modules changes looks for types only inside stdlib (because space which is passed to it on the construction step is stdlib space).

I am thinking about adding &top to the stdlib as a workaround for the first issue and passing top space to the get-type as a fix for the second one. But if we could get space where atom came from (or call came from) then it would fix both get-doc and #354.

@luketpeterson
Copy link
Contributor

Sorry I misunderstood. It's basically environment-capture (closure) behavior that we want.

@luketpeterson luketpeterson changed the title include operation needed to access external functionality within the metta module's scope case cannot be used with function calls defined out of the case metta module scope Mar 28, 2024
@luketpeterson
Copy link
Contributor

luketpeterson commented Mar 29, 2024

I am thinking about adding &top to the stdlib...

With the fix in #643 you can access !(mod-space! top) without changing the stdlib. I know that's not a fix for this issue, but slightly more convenient than adding to stdlib.

@luketpeterson
Copy link
Contributor

luketpeterson commented Mar 29, 2024

Part 1: I want to describe what’s happening (I know you already know this @vsbogd , but this will help synchronize terms for the ongoing discussion)

  • The case op atom has an internal pointer to the module space it belongs to. So case inside the synthesize module has one space, and the runner’s top module (bug.metta) has a different space.

  • Importing the synthesize function from the synthesize module imports that case op-atom with its ref to the synthesize module space.

  • The axiom function is defined in the top space, so the query in the synthesize module space doesn’t find it.

Part 2: what solution do we want - from a language usability perspective?

  • We probably don’t just want to have the case op run the query in the calling space (top), as that will create different problems if the module intended to use atoms in the module’s sub-space.

  • My thought was that it would be nice if the evaluation was not part of case, but I see this comment

    // Interpreting argument inside CaseOp is required because otherwise `Empty` result

  • My next thought was a wrapper that would wrap an atom and capture the current space. I had to rearrange the parentheses in the example to control the order of evaluation, but the updated example looks like this:

synthesize.metta

;; Define synthesize
(: synthesize (-> Atom Atom Atom))
(= (synthesize $query $axiom) (case $axiom (($query $query))))

bug.metta

;; Import synthesize
!(import! &self synthesize)

;; Define axiom
(: axiom (-> Atom))
(= (axiom) (: f (-> Number String)))

;; Test synthesize
!(synthesize (: $term $type) (capture (axiom)))

You can try it out with #644

@luketpeterson
Copy link
Contributor

If we like this pattern but not the name "capture", other options are "close", "enclose", "context", "ctx", etc.

@vsbogd
Copy link
Collaborator Author

vsbogd commented Mar 29, 2024

My thought was that it would be nice if the evaluation was not part of case, but I see this comment

It is absolutely equal to my understanding and minimal MeTTa allows doing this without evaluating expressions inside case or assert. Instead case or assert can return the atom which calls interpreter before calling matching part of the procedure. I will prepare PR after finishing docs PR to illustrate the idea.

vsbogd added a commit that referenced this issue Apr 11, 2024
Adding "capture" operation.  Experimental approach to issue #354
@vsbogd
Copy link
Collaborator Author

vsbogd commented Apr 11, 2024

I merged #644 because it introduces a nice functionality. On the other hand to me it still doesn't resolve the issue completely because to use this solution the user should pass the context manually. I was thinking about passing the context automatically but I am not sure what is the best way to implement it.

@luketpeterson
Copy link
Contributor

to use this solution the user should pass the context manually. I was thinking about passing the context automatically but I am not sure what is the best way to implement it.

I'm not sure that's always desirable to automatically capture the caller's context. Because it might make it impossible to implement functions that are intended to work within the context of the sub-module.

@vsbogd
Copy link
Collaborator Author

vsbogd commented Apr 18, 2024

it might make it impossible to implement functions that are intended to work within the context of the sub-module.

It is interesting case but usually passed function gets all inputs from the caller via arguments, thus I would say this case is rare. May be we could vice versa add a manual way to make functions which can be embedded into foreign context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants