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

Bug or User error? when using frames #21

Open
linas opened this issue Jun 18, 2023 · 3 comments
Open

Bug or User error? when using frames #21

linas opened this issue Jun 18, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@linas
Copy link
Member

linas commented Jun 18, 2023

Here's some weird unexpected, confusing behavior w.r.t. frames. It's both a bug and a user-error. It is exhibited by the following:

(use-modules (opencog) (opencog exec))
(use-modules (opencog persist))
(use-modules (opencog persist-rocks))

(define a (AtomSpace))
(define b (AtomSpace))
(define c (AtomSpace a b))
(cog-set-atomspace! a)
(Concept "foo")
(Concept "bar")
(Concept "baz")
(Concept "I'm in A")

(cog-set-atomspace! c)
(define rsn (RocksStorageNode "rocks:///tmp/foo"))
(cog-open rsn)

; Oh no! Saving atomspace contents without saving frames first!!
(store-atomspace)
(cog-rocks-print rsn "")

; Now store atomspace b (which should be empty!!)
(store-frames b)

; Hmm looks like all atoms got assigned to atomspace b! Oh no!
(cog-rocks-print rsn "")

If the store-frames is done first, before the store-atomspace, then the correct behavior is seen. This is a user-error, since the user should have done the store-frames first. It is also a bug, since the store-frames should not have re-assigned the atoms to the wrong atomspace.

@linas linas added the bug Something isn't working label Jun 18, 2023
@sikefield3
Copy link

; Hmm looks like all atoms got assigned to atomspace b! Oh no!
(cog-rocks-print rsn "")

Maybe my understanding of this is wrong, but shouldn't RocksDB stay connected with the atomspace it was created in (in this case AS 'C') and so the content of atomspace 'A' is still in the DB.

If I just print the AS B:

(cog-set-atomspace! b)
(cog-prt-atomspace)

, I get nothing (as expected).

@linas
Copy link
Member Author

linas commented Jun 27, 2023

Sure! Let me walk through the example, and some of the design choices (which could be faulty, but seemed like the "right thing to do", at the time.)

  • The RocksStorageNode is meant to be a handle holding a URL, and "nothing more". We have two choices: tie it to the atomspace in which it is created, or leave it untied, a free-floating handle that is "always" available. The second seemed better, because maybe one creates atompsaces d,e,f later, and one might want to store those, too. Thus, it seemed better to leave it free-floating.
  • Same idea for cog-open. It seems best to leave the connection free-floating, so that I can store d,e,f in it, later on, instead of always having it be bound up with atomspace c.

OK, so far, so good. Now, when cog-open is called, it prints

Rocks: DB-version=2 multi-space=0 initial aid=1

The initial-aid=1 basically says that the storage node is empty. The multi-space=0 thing says that it is initialized to hold only one atomspace; that the concept of multiple frames hasn't been turned on, yet. Now, we could have noticed that rsn is in c, and initialized it to be multi-space=true. But this creates problems:

  • What if the user don't want to store multiple spaces? What if the user just wanted to store everything flattened down to one? How would that work?
  • What if the rsn wasn't empty (and was a mono-space, i.e. had no frames)? Should we automatically promote it to be multi-space? Yuck .. if we were opening for read-only, we should not be corrupting it's contents. But we don't know if we're opening for read-only, so ...

So, it seemed best to just open it, and leave however we found it, i.e. in this case, leave it as monospace.

OK, so far, so good. Now comes the store-atomspace. What should that do? Should it go "a ha, I'm in some non-trivial frame, and therefore I should switch to multi-space mode, and store the frames, before doing anything else?" Or should it just store the current atomspace, as instructed, (in mono-space mode) without trying to second-guess the user's intent? The second choice seemed best, since, if the user wanted to store frames, they should have said so.

So ... the current atomspace gets stored, but it gets stored in monospace mode. Which seems fine. Its a minimalist, simple thing to do.

Next is (store-frames b) -- this is arguably a user-error. It tells the systems to switch from monospace mode to multi-space mode. Fine. But it then tags all the atoms in rocks with frame b (without realizing that "in reality", it's "empty") Note that if instead the user had said (store-frames c) at this point, it would have thrown an error: when switching from monospace mode to multi-space, all the atoms have to be assigned a space: but which one? a, b or c? It would be horrible to try to start guessing, by comparing rocks contents to atomspace contents, matching and pairing them up, sorting them into buckets. Yuck, a real mess that gets messier the more complicated the example. Easier to throw an error.

So now we arrive to the present moment. You are right, that if you print b, you will see that it is empty. And that is as it should be. But now try this:

(store-frames c)
(store-atomspace)
(cog-rocks-print rsn "")

You will now see that rocks has atoms in both a and in b, which might be "surprising" to the user. It might not be what they were expecting. The print shows this:

rkey: >>a@4:<<    rval: >>(ConceptNode "foo")<<     ;; foo has UID 4
rkey: >>d@7<<    rval: >>(as "(uuid . 3)")<<        ;; atomspace b as uid 7
rkey: >>d@8<<    rval: >>(as "(uuid . 2)")<<        ;; atomspace a has uid 8
rkey: >>d@9<<    rval: >>(as "(uuid . 4)" 8 7)<<    ;; atomspace c has uid 9
rkey: >>k@4:7:+1<<    rval: >><<                    ;; foo (uid 4) is in uid 7 aka atomspace b
rkey: >>k@4:8:+1<<    rval: >><<                    ;; foo (uid 4) is in uid 8 aka atomspace a

If you closed, exited, restarted, and loaded, you would now get "foo" in both a and b, and b would not be empty. That's the surprise.

Overall, the whole thing feels like a user-error, to me. The code could have done something else, instead, but I don't really see any good reason to "fix" anything, just right now. The alternatives seem worse.

@linas
Copy link
Member Author

linas commented Jun 27, 2023

So, I'm thinking this is a user error. There are two choices:

  • Do nothing. "working as designed", its a weird corner case, and the user shot themselves in the foot.
  • Throw an error: something that says "rocks already contains atoms in monospace format; you cannot upgrade to multi-space mode."

The second choice is icky. Suppose we created a rocks monospace store a few months ago. Now we want to add a few layers on top of it. It would be nice to be able to "just do it", instead of getting an error.

So I'm tempted to close this as "user error, working as designed".

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

No branches or pull requests

2 participants