-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Clean up settings properly for removed models and also when user manu… #2098
Conversation
So now if a user temporarily moves/renames a .gguf file or changes their model directory, GPT4All will permanently forget the settings the user has configured for whichever models it doesn't see the next time it launches? That doesn't seem like a good thing. |
I disagree. We cannot control what a user does in all circumstances outside of the GUI. If the user moves files around manually we have no way of associating the old settings with that new location. The settings are useless in that case. We should we keep them? What proposed behavior would you have instead? The only other thing I can think to do would be to just ignore those settings but not erase them. In other words, just skip over them without the erase... then if a user somehow restores the file to the proper place they become relevant again? Seems we're coding for a very very rare eventuality in that case and the settings file can get larded up with old settings. I guess I could be convinced to do it this way but the OCD in me really hates that we have no way to clean up the settings file in that case. |
In the settings of the GUI, there is the "restore defaults" button, which I would expect something to do like this, but the restore defaults button only works per model, not global. There is also the "remove" button, but that one also only removes a single model settings.
|
This is only for clones...
Yeah, no good.
Doesn't help because the remove button is only for clones.
Yeah, this still doesn't help with the stale settings from models that have been removed/renamed. The only option I can think of is again:
I'll wait for @cebtenzzre to comment if he has another idea... |
Simply not listing models that don't exist--or at least, listing them with a non-blank filename and an indication that they are not available--would be consistent with previous versions of GPT4All. IMO, deleting settings is a destructive option that should only be done if the user explicitly requests it--whether they click a "remove" button, or acknowledge a warning dialog ("three model files were not found, would you like to remove them from your configuration?"), or click a "cleanup model settings" button, or anything else like that. I will always be frustrated by software that tries to be smart and ends up creating more work for me. Perhaps I start GPT4All and forgot to plug in the external hard drive I keep my model files on. Then upon restarting it with the drive plugged in, all of my settings are gone. That would be a bug to me. It is wrong to assume that just because a file is not currently present at a particular path that it is unlikely to appear again. |
Ok, then I will make it so that the setting is ignored and not listed if it doesn't have a corresponding valid file, but I won't erase it. |
…ally deletes. Signed-off-by: Adam Treat <treat.adam@gmail.com>
Signed-off-by: Adam Treat <treat.adam@gmail.com>
bd1ad09
to
79ada17
Compare
The title of this PR wasn't updated before it was merged, but in its current form the stale entries will simply be ignored instead of deleted, for the reasons discussed above. |
…ally deletes.