Skip to content

fix: correct default value for max_memory.#284

Merged
p-e-w merged 2 commits intop-e-w:masterfrom
AWuhrmann:patch-1
Apr 8, 2026
Merged

fix: correct default value for max_memory.#284
p-e-w merged 2 commits intop-e-w:masterfrom
AWuhrmann:patch-1

Conversation

@AWuhrmann
Copy link
Copy Markdown
Contributor

This is just a typo / quick fix.

The other does not compile.

The other does not compile.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request corrects a TOML syntax error in a commented-out example within config.default.toml by replacing colons with equals signs in an inline table. The review feedback suggests applying this same correction to the configuration description in src/heretic/config.py for consistency and recommends adding spaces within the curly braces to improve readability.


# Maximum memory to allocate per device.
# max_memory = {"0": "20GB", "cpu": "64GB"}
# max_memory = {"0" = "20GB", "cpu" = "64GB"}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The fix correctly addresses the TOML syntax error in the example (inline tables in TOML must use = for key-value pairs). However, the same incorrect example (using : instead of =) is present in the description of the max_memory field in src/heretic/config.py (line 122). To ensure consistency and correct documentation/CLI help, that file should also be updated.

Additionally, adding spaces inside the curly braces is more idiomatic for TOML inline tables and improves readability.

Suggested change
# max_memory = {"0" = "20GB", "cpu" = "64GB"}
# max_memory = { "0" = "20GB", "cpu" = "64GB" }

@p-e-w
Copy link
Copy Markdown
Owner

p-e-w commented Apr 7, 2026

Thanks! See the suggestion from Gemini.

@p-e-w p-e-w merged commit a1a1c30 into p-e-w:master Apr 8, 2026
4 checks passed
@p-e-w
Copy link
Copy Markdown
Owner

p-e-w commented Apr 8, 2026

Merged, thanks!

@hdnh2006
Copy link
Copy Markdown

--max-memory parameter is not working for me, I am writing it like this:

--max-memory { "0" = "8GB", "cpu" = "20GB" }

I am getting this:
heretic: error: unrecognized arguments: 0 = 8GB, cpu = 20GB }

Am I missing something?

@p-e-w
Copy link
Copy Markdown
Owner

p-e-w commented Apr 10, 2026

That's not valid shell syntax. You probably need to wrap everything in quotes, but even that may not be enough.

@AWuhrmann
Copy link
Copy Markdown
Contributor Author

Yes, if I remember correctly wrapping it in single quotes does work.

@hdnh2006
Copy link
Copy Markdown

hdnh2006 commented Apr 11, 2026

That's not valid shell syntax. You probably need to wrap everything in quotes, but even that may not be enough.

You right! this worked for me:
--max-memory '{ "0" = "8GB", "cpu" = "20GB" }'

thanks

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.

3 participants