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

Fix unique #785

Merged
merged 3 commits into from
Oct 23, 2024
Merged

Fix unique #785

merged 3 commits into from
Oct 23, 2024

Conversation

vsbogd
Copy link
Collaborator

@vsbogd vsbogd commented Oct 21, 2024

Fixes #760
Also removes interpreter from the functions implementations.

Remove internal interpreter call and fix type to interpret arguments
before passing to the operation.
Rename old implementations to ...-atom, add MeTTa implementations of
union, intersection, unique and subtraction with non-deterministic input
and output.
@vsbogd vsbogd requested a review from Necr0x0Der October 21, 2024 15:21
@vsbogd
Copy link
Collaborator Author

vsbogd commented Oct 21, 2024

@ngeiswei could you please check if it what you needed?

@ngeiswei
Copy link
Contributor

Works like a charm, thanks!

ngeiswei
ngeiswei previously approved these changes Oct 23, 2024
@ngeiswei
Copy link
Contributor

Unfortunately I spoke too fast :-(, it does not work in my case, but it is a complex one, I want to simplify it a bunch before pasting it here. But to give you an idea, the following

!(unique (bc &kb
             (fromNumber 5)
             (: $prg (-> (: $x (SongIn "English")) (SongIn "Chinese")))))

does not work, while the following

!(let* (($results (collapse
                   (bc &kb
                       (fromNumber 5)
                       (: $prg (-> (: $x (SongIn "English")) (SongIn "Chinese"))))))
        ($uniq-results (unique (superpose $results))))
   $uniq-results)

does. Meaning that I need to explicitly collapse the results, then call unique on their superposition, and then it works.

I am not sure if it is a bug or a feature, I can still use that, but of course it is not as elegant as just calling unique on my non-deterministic function call.

@ngeiswei
Copy link
Contributor

Please find a simplified example

(= (f) a)
(= (f) a)

!(unique (f))
!(let $y (collapse (f)) (unique (superpose $y)))

@ngeiswei ngeiswei dismissed their stale review October 23, 2024 06:22

I do not approve anymore

@vsbogd
Copy link
Collaborator Author

vsbogd commented Oct 23, 2024

Thanks @ngeiswei. Seems like it is a bug which prevented me to implement it last time. unique is implemented in stdlib and thus when collapse is called from implementation it uses stdlib space as a context. As consequence (f) is not evaluated and stays (f) because stdlib doesn't have definition of (f). It is the reason why unique returns (f) and after collapse it is evaluated in two instances of a.

@ngeiswei
Copy link
Contributor

This is better than nothing, I can still use it by collapsing and superposing, so I approve merging if that's the best we can get for now.

@vsbogd
Copy link
Collaborator Author

vsbogd commented Oct 23, 2024

Well, I think we can merge it, because it is better then previous state. Then I will think how to fix it properly and raise another PR.

@vsbogd vsbogd merged commit 27ce1cd into trueagi-io:main Oct 23, 2024
2 checks passed
@vsbogd
Copy link
Collaborator Author

vsbogd commented Oct 23, 2024

Btw, @ngeiswei , the following code works:

(= (f) a)
(= (f) a)

!(unique (capture (f)))

@vsbogd vsbogd deleted the fix-unique branch November 18, 2024 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subtraction, union, intersection and unique doesn't work after switching to minimal MeTTa
2 participants