cheshire-cat-ai / docs

Documentation for the Cheshire Cat AI
https://cheshire-cat-ai.github.io/docs/
34 stars 28 forks source link

Error on example of hook save settings plugin #155

Closed ElioErrico closed 4 months ago

ElioErrico commented 4 months ago

Hello Nicola, I found this little error in Hook save settings documentation:

https://cheshire-cat-ai.github.io/docs/technical/plugins/hooks/#__tabbed_1_4

Im speaking of hook save settings plugin.

The example is

@plugin
def save_settings(settings):
    # overwrite your settings Dict values
    settings.episodic_memory_k = 6
    settings.episodic_memory_threshold = 0.9
    settings.declarative_memory_k = 5
    return settings

Should be:

@plugin
def save_settings(settings):

    # overwrite your settings Dict values
    settings[episodic_memory_k] = 6
    settings[episodic_memory_threshold] = 0.9
    settings[declarative_memory_k] = 5

    return settings
lucagobbi commented 4 months ago

You're right: that throws an error, because settings is of type Dict. Nevertheless, that example is quite misleading from my pov, because save_settings and load_settings are hook provided to completely override the way you save and load plugin settings (e.g. storing them in a separate DB and serialize them in a format compatible with the Cat once read from that DB).

Even with your example (btw, you're missing quotes " in dict access):

@plugin
def save_settings(settings):

    # overwrite your settings Dict values
    settings["episodic_memory_k"] = 6
    settings["episodic_memory_threshold"] = 0.9
    settings["declarative_memory_k"] = 5

    return settings

settings will not be saved because you're simply returning them and if you look at the core you'll see that it is directly returning the hook function and bypass the default saving strategy.

I suggest to avoid doing something like this:


ccat = CheshireCat()

@plugin
def save_settings(settings):

    # overwrite your settings Dict values
    settings["episodic_memory_k"] = 6
    settings["episodic_memory_threshold"] = 0.9
    settings["declarative_memory_k"] = 5
    settings = ccat.mad_hatter.get_plugin().save_settings(settings)

    return settings

I don't know why sometimes I get it to work, even if it is nosense. If you look again at the code in the core, the code that I've just provided should fall in an infinite loop and that is actually happening to me most of the time.

@pieroit @Pingdred @valentimarco @nicola-corbellini am i missing something?

I'm working on a PR to update the hooks documentation with the latest changes. Maybe we should also include more detailed examples for these hooks.

ElioErrico commented 4 months ago

You are right.

What I did in order to save new settings after having modified it inside the hook is calling a function (always inside the hook) in order to save the updated settings dictionary in settings.json.

This function basically updates settings.json file in the plugin folder.

I run the function before returning settings in the hook.

Don't know if clear.