Repository navigation
Conversation
Signed-off-by: awphi <adam.hodson@arm.com>
Signed-off-by: awphi <adam.hodson@arm.com>
Signed-off-by: awphi <adam.hodson@arm.com>
|
question: does this need to be run with the experimental flag enabled? |
|
I tried this. I have the experimental feature in my logs. I get this error: I tried again with the experimental flag on, and it worked |
Yep that's expected while we keep the feature experimental! It's very likely the schema for binding a parameter to a fact is going to change. We've come up with a new design even today that's a bit more flexible so this is probably for the best for now. It does mean this PR probably should not be merged until the feature is more stable. I'll make a note in the PR |
| candidates.sort() | ||
| if not memory: | ||
| for quality, url in candidates: | ||
| if QUANTS[quality] == "Q4_K_M": | ||
| print(url) | ||
| return | ||
| raise ValueError("Q4_K_M is unavailable") | ||
|
|
||
| total = int(memory) * 1024 | ||
| # Reserve RAM for the OS and background processes. | ||
| budget = total - max(4 * 1024**3, min(int(total * 0.15), 32 * 1024**3)) |
There was a problem hiding this comment.
can we move the candidates.sort() down so the loop is more understandable?
| candidates.sort() | |
| if not memory: | |
| for quality, url in candidates: | |
| if QUANTS[quality] == "Q4_K_M": | |
| print(url) | |
| return | |
| raise ValueError("Q4_K_M is unavailable") | |
| total = int(memory) * 1024 | |
| # Reserve RAM for the OS and background processes. | |
| budget = total - max(4 * 1024**3, min(int(total * 0.15), 32 * 1024**3)) | |
| if not memory: | |
| for quality, url in candidates: | |
| if QUANTS[quality] == "Q4_K_M": | |
| print(url) | |
| return | |
| raise ValueError("Q4_K_M is unavailable") | |
| total = int(memory) * 1024 | |
| # Reserve RAM for the OS and background processes. | |
| budget = total - max(4 * 1024**3, min(int(total * 0.15), 32 * 1024**3)) | |
| candidates.sort() |
| if context <= 0 or (memory and int(memory) <= 0): | ||
| raise ValueError("TARGET_MEMORY_KIB and CONTEXT_LENGTH must be positive") |
There was a problem hiding this comment.
- the error message is confusing. Reading the code, it seems like
TARGET_MEMORY_KIBis optional whileCONTEXT_LENGTHis necessary. The error message sounds like we need bothTARGET_MEMORY_KIBandCONTEXT_LENGTHto proceed, which is not true - The if condition doesn't really represent the error message either. With current logic, the error message won't show if
TARGET_MEMORY_KIBis not set ifCONTEXT_LENGTHis set and positive - It seems like the check for value of memory is happening again in line 31. I recommend removing check of the memory here
| 1. **Arm Hardware**: An Arm system (physical or virtual). | ||
| 2. **Docker**: For container orchestration with Topo | ||
| 3. **LLM Model**: Optional when overriding the bundled default; provide a supported single-file GGUF model (e.g., Llama 3.1, Mistral, etc.) | ||
| 3. **Topo**: An experimental build supporting `configure --target` and parameter fact bindings. |
There was a problem hiding this comment.
Can we have a better description here? Technically, it's not an experimental build. We just need to turn on the experimental flag on the normal release build
|
|
||
| ```bash | ||
| cd topo-lama-web-ui | ||
| cd topo-llama-web-ui |
Demos features introduced in arm/topo#558 to demo target-aware configuration:
topo configure|clone --target(or manually typed if using a Topo version without this feature)NOTE: This PR demonstrates (but does not wholly rely on) a feature gated behind
TOPO_EXPERIMENTAL_FEATURES=1;topo configure --target. It's best not to merge this until that feature is finalized and generally available.