ComfyUI: Remove --normalvram#1644
Conversation
Removed 'normalvram' option
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the --normalvram argument from ComfyUI's configuration options and initial value selection logic. Feedback identifies a critical backward compatibility issue: removing the option from the UI does not clear it from existing user configurations, which will cause ComfyUI to crash upon encountering the now-unknown argument. It is recommended to filter this flag out during the package execution process. Additionally, the GPU detection logic is susceptible to an InvalidOperationException if no GPUs are found, which should be mitigated by providing a default value before calling Max().
|
Added stripping of existing --normalvram argument from settings.json upon settings load and atomic rebuild. Launch argument removed from comfy configuration and covers users with pre-existing set arg from either manually set, or when SM set it automatically for 4-8GB GPU during install. |
| } | ||
| } | ||
|
|
||
| private bool NormalizeLoadedSettings(Settings settings) |
There was a problem hiding this comment.
I think the Normalized settings addition to SettingsManager might be the wrong place for this logic? Seems like if a package requires a migration to settings before proceeding, that should be done in the package itself prior to launch or install.
Since I don't see why this has to be done right when the settings json itself is actually deserialized.
Even if it needs to be applied right away, perhaps hooking onto the SettingManager's "EventHandler? LibraryDirChanged" within the specific type needing the change would be cleaner. But in this case again it might be something that can be deferred until launch or updates?
There was a problem hiding this comment.
Moved into ComfyUI/C-Zluda launch path.
Launch arg removal from settings is now just logged to file instead of also in console.
- Removed obsolete SettingsManager migration test. Refactored it into standalone package-level test.
2 participants
--normalvram has been removed from ComfyUI as of this PR and will throw unknown argument error at package launch if the user has it enabled in Launch Options window as it appears there.
It is functionally implied by default by ComfyUI unless it is overridden by the user or by certain hardware checks by Comfy's internal logic.
Since SM sets normalvram automatically on installs with 4-8GB vram GPUs, any user before this change that has their comfy install using the repo commits instead of releases, their comfy launch will hit a launch argument error.