-
Notifications
You must be signed in to change notification settings - Fork 0
New huge version #9
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
Changes from all commits
e68d338
4d62707
05d7bdd
897c0d1
7d4258a
604dc10
0329ab4
272177b
ebb2228
5ba6b26
2268aa0
9b1cf09
898df91
3c374a8
0a18abe
0af15ba
34f98cb
00d4c37
089a951
cabb335
7184183
1074dfb
e7b7900
1ba714b
abf0a1c
99ce0b6
44590a1
f24ab6b
ad2056b
7f13228
317ae7c
09d3f82
c50ca28
05471e3
073ccf2
c7d1f58
f7cc293
5695047
241a24c
62c7d57
2c23864
1256b3d
fa88120
d18b2e1
979a003
6028e60
1acdd70
2e90881
a15727e
2239957
df661c9
ea163f2
b06eeec
aa9b707
90e5d18
9635a97
040f19a
e13ae38
a98c559
ab5b659
831e7d3
2661aaf
c348a98
b3cdc73
15a80c8
bee84ef
9494384
539fb57
0a32982
9cc6037
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| /config.local | ||
| /tmp | ||
| /cache |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| [cache] | ||
| type = hardlink | ||
| protected = true | ||
| [core] | ||
| remote = storage | ||
| ['remote "nas-remote"'] | ||
| url = //diskstation_105/dvc_storage | ||
| ['remote "storage"'] | ||
| url = s3://dvc-storage | ||
| endpointurl = https://minio.komolov.synology.me | ||
| region = us-east-1 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| # Add patterns of files dvc should ignore, which could improve | ||
| # the performance. Learn more at | ||
| # https://dvc.org/doc/user-guide/dvcignore |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,37 +1,72 @@ | ||
| repos: | ||
| - repo: https://github.com/pre-commit/pre-commit-hooks | ||
| rev: v4.5.0 | ||
| rev: v4.6.0 | ||
| hooks: | ||
| - id: check-ast | ||
| description: Check Python AST syntax | ||
| - id: check-case-conflict | ||
| description: Check for case conflicts in filenames | ||
| - id: check-toml | ||
| description: Validate TOML files | ||
| - id: check-yaml | ||
| description: Validate YAML files | ||
| args: ['--unsafe'] # Allow custom YAML tags used by Hydra | ||
| - id: check-json | ||
| description: Validate JSON files | ||
| - id: detect-private-key | ||
| description: Detect private keys in code | ||
| - id: end-of-file-fixer | ||
| exclude: '.*\.sh$' | ||
| description: Fix missing newlines at end of files | ||
| exclude: '.*\.(sh|bat)$' | ||
| - id: trailing-whitespace | ||
| description: Remove trailing whitespace | ||
| exclude: '.*\.(md|rst)$' # Preserve markdown line breaks | ||
| - id: check-added-large-files | ||
| description: Check for large files being committed | ||
| args: ['--maxkb=10240'] # Allow up to 10MB files | ||
| - id: check-merge-conflict | ||
| description: Check for merge conflict markers | ||
| - id: debug-statements | ||
| description: Check for debug statements (pdb, etc.) | ||
| language: python | ||
| - id: mixed-line-ending | ||
| description: Check for mixed line endings | ||
|
|
||
| - repo: https://github.com/astral-sh/ruff-pre-commit | ||
| rev: v0.1.5 | ||
| rev: v0.8.4 | ||
| hooks: | ||
| - id: ruff | ||
| args: [--fix, --exit-non-zero-on-fix, --show-fixes, --select, I] | ||
| name: ruff-linter | ||
| description: Run Ruff linter with auto-fixes | ||
| args: [--fix, --show-fixes, --config=pyproject.toml] | ||
| - id: ruff-format | ||
| name: ruff-formatter | ||
| description: Run Ruff formatter | ||
| args: [--config=pyproject.toml] | ||
|
|
||
| - repo: https://github.com/psf/black | ||
| rev: 23.1.0 | ||
| rev: 24.10.0 | ||
| hooks: | ||
| - id: black | ||
| args: [--fast] | ||
| - repo: https://github.com/PyCQA/flake8 | ||
| rev: 5.0.0 | ||
| name: black-formatter | ||
| description: Format Python code with Black | ||
| language_version: python3 | ||
|
|
||
|
|
||
| - repo: https://github.com/pycqa/bandit | ||
| rev: 1.7.10 | ||
| hooks: | ||
| - id: bandit | ||
| name: bandit-security-checker | ||
| description: Run Bandit security linter | ||
| args: [-c, pyproject.toml] | ||
| exclude: ^(tests/|legacy) | ||
|
|
||
| - repo: https://github.com/codespell-project/codespell | ||
| rev: v2.3.0 | ||
| hooks: | ||
| - id: flake8 | ||
| additional_dependencies: [pep8-naming, flake8-bugbear ] | ||
| # - repo: https://github.com/pre-commit/mirrors-mypy | ||
| # rev: 'v1.15.0' | ||
| # hooks: | ||
| # - id: mypy | ||
| # args: [ --strict ] | ||
| - id: codespell | ||
| name: codespell-spell-checker | ||
| description: Check for common misspellings | ||
| args: [--write-changes] | ||
| exclude: ^(\.git/|docs/_build/|models/|data/|README.md) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| FROM nvidia/cuda:12.1.0-cudnn8-runtime-ubuntu22.04 | ||
| FROM nvidia/cuda:12.4.1-runtime-ubuntu22.04 | ||
|
|
||
| ENV DEBIAN_FRONTEND=noninteractive | ||
|
|
||
|
|
@@ -16,8 +16,12 @@ RUN apt-get update && \ | |
| build-essential \ | ||
| cmake \ | ||
| curl \ | ||
| wget \ | ||
| jq \ | ||
| libcurl4-openssl-dev \ | ||
| apt-transport-https \ | ||
| ca-certificates \ | ||
| gnupg \ | ||
| && \ | ||
| rm -rf /var/lib/apt/lists/* | ||
|
|
||
|
|
@@ -27,24 +31,40 @@ RUN python3.11 -m ensurepip && python3.11 -m pip install --upgrade pip | |
|
|
||
| RUN pip install --upgrade --ignore-installed wheel==0.45.1 | ||
|
|
||
| WORKDIR /training_model | ||
| COPY pyproject.toml poetry.lock ./ | ||
| RUN pip install poetry && poetry config virtualenvs.create false && poetry install --no-root --only main | ||
|
|
||
| COPY training_model ./training_model | ||
| COPY testing_model ./testing_model | ||
| COPY data ./data | ||
| COPY main.py . | ||
| # Install Docker CLI so the training container can control Docker to run the RKLLM converter | ||
| RUN mkdir -p /etc/apt/keyrings && \ | ||
| curl -fsSL https://download.docker.com/linux/ubuntu/gpg | gpg --dearmor -o /etc/apt/keyrings/docker.gpg && \ | ||
| echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.gpg] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable" \ | ||
| > /etc/apt/sources.list.d/docker.list && \ | ||
| apt-get update && apt-get install -y docker-ce-cli && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| WORKDIR /llama.cpp | ||
| RUN git clone https://github.com/ggml-org/llama.cpp.git . && \ | ||
| cmake -B build && \ | ||
| cmake --build build --config Release | ||
|
|
||
| WORKDIR /training_model | ||
|
|
||
| WORKDIR /app | ||
| COPY pyproject.toml poetry.lock ./ | ||
| RUN pip install poetry && \ | ||
| pip install pyyaml && \ | ||
| poetry lock --no-interaction --no-ansi || true && \ | ||
| poetry install --no-root --only main --no-interaction --no-ansi | ||
|
Comment on lines
+48
to
+52
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Poetry lock failure silently ignored. Line 51 uses Consider handling lock failures more explicitly: # Try to use existing lock, but allow regeneration if corrupted
RUN pip install poetry && \
pip install pyyaml && \
(poetry lock --check || poetry lock --no-interaction --no-ansi) && \
poetry install --no-root --only main --no-interaction --no-ansi🤖 Prompt for AI Agents |
||
|
|
||
| # Create symlink for llama.cpp so training code can find it at expected relative path | ||
| RUN ln -s /llama.cpp ./llama.cpp | ||
|
|
||
| # Create symlink for quantize executable to match Windows naming convention | ||
| RUN ln -s /llama.cpp/build/bin/llama-quantize /app/llama.cpp/llama-quantize.exe | ||
|
Comment on lines
+57
to
+58
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify .exe extension requirement on Linux. The symlink creates Verify whether training_model code hardcodes the .exe extension or if this can be made platform-aware: 🏁 Script executed: #!/bin/bash
# Check for hardcoded .exe references in training code
rg -n "llama-quantize\.exe" training_model/Length of output: 218 I need to examine the broader context around these hardcoded references to see if there's platform detection logic or if the parameter can be overridden: Length of output: 924 🏁 Script executed: #!/bin/bash
# Check function signature and how quantized_path is used
ast-grep --pattern $'def $_($$$quantized_path$$$) {
$$$
}'Length of output: 0 🏁 Script executed: #!/bin/bash
# Search for platform detection or conditional logic related to quantize
rg -n -i "platform|windows|linux|os\." training_model/one_file_train.py | head -20Length of output: 1039 I need to check how this function is called to see if the parameter is being overridden or if it relies on the default: Length of output: 138 🏁 Script executed: #!/bin/bash
# Check if there are any tests or other references to the quantized_path parameter
rg -n "quantized_path\s*=" training_model/ --type=pyLength of output: 102 🏁 Script executed: #!/bin/bash
# Look for platform detection at top level or imports
head -50 training_model/one_file_train.py | grep -n "import\|platform\|sys"Length of output: 906 I need to check what the config actually contains for the quantized_path value: Length of output: 1646 🏁 Script executed: #!/bin/bash
# Look for config file or config defaults
fd -type f -name "*.yaml" -o -name "*.yml" -o -name "*config*" training_model/ 2>/dev/null | head -10Length of output: 0 🏁 Script executed: #!/bin/bash
# Check context around line 1836-1841 to see full function call
sed -n '1830,1845p' training_model/one_file_train.pyLength of output: 761 Let me search for the config definition at the project level: Length of output: 105 🏁 Script executed: #!/bin/bash
# Search for where cfg.paths.quantized_path is defined
rg -n "quantized_path.*=" . --type=py --type=yaml | grep -v "^training_model"Length of output: 383 🏁 Script executed: #!/bin/bash
# Look in parent dirs and common config locations
rg -n "paths:" . --type=yaml -A5 | head -30Length of output: 313 Length of output: 604 🏁 Script executed: #!/bin/bash
# Look for quantized_path in entire config.yaml
cat ./conf/config.yaml | grep -n "quantized_path" -B2 -A2Length of output: 268 Fix hardcoded The
The Dockerfile symlink on line 58 is a workaround for these hardcoded assumptions. Either make the naming truly platform-aware throughout or document that 🤖 Prompt for AI Agents |
||
|
|
||
| COPY training_model ./training_model | ||
| COPY testing_model ./testing_model | ||
| COPY evaluation ./evaluation | ||
| COPY data ./data | ||
| COPY conf ./conf | ||
| COPY main.py . | ||
| COPY run_pipeline.sh . | ||
|
|
||
| RUN chmod +x run_pipeline.sh | ||
|
|
||
| CMD ["./run_pipeline.sh"] | ||
| #CMD python -m training_model | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,12 +2,12 @@ FROM ollama/ollama:latest | |||||
|
|
||||||
| RUN apt-get update && apt-get install -y curl | ||||||
|
|
||||||
| VOLUME ["/root/.ollama", "/training_model/models"] | ||||||
| RUN mkdir -p /training_model/models && chmod 777 /training_model/models | ||||||
| VOLUME ["/root/.ollama", "/app/models"] | ||||||
| RUN mkdir -p /app/models && chmod 777 /app/models | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overly permissive directory permissions pose a security risk. Setting permissions to Apply this diff to use more restrictive permissions: -RUN mkdir -p /app/models && chmod 777 /app/models
+RUN mkdir -p /app/models && chmod 755 /app/modelsIf write access by the Ollama process is required, consider using 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| EXPOSE 11434 | ||||||
|
|
||||||
| ENV OLLAMA_MODELS="/training_model/models" | ||||||
| ENV OLLAMA_MODELS="/app/models" | ||||||
|
|
||||||
| COPY entrypoint.sh /usr/local/bin/entrypoint.sh | ||||||
| RUN chmod +x /usr/local/bin/entrypoint.sh | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Docker-in-Docker pattern requires security consideration.
Installing Docker CLI allows the training container to control the host Docker daemon (typically via mounted
/var/run/docker.sock). This grants the container significant privileges and should be carefully controlled.Ensure that:
If docker-compose.yaml mounts the socket, verify it's conditionally enabled:
🏁 Script executed:
Length of output: 131
🏁 Script executed:
Length of output: 456
🏁 Script executed:
Length of output: 1337
🏁 Script executed:
Length of output: 420
Docker socket mount is unconditional and should be restricted to converter operations only.
The socket mount on
llm_trainingservice (docker-compose.yaml line 47) is enabled unconditionally with full read-write access, despite the Dockerfile comment stating it's only needed for RKLLM converter. The converter runs as a separate gated service (profiles: ["converter"]) but lacks the socket mount, creating an inconsistent and over-privileged setup.Fix required:
rkllm_converterservice (with conditional profile), ORllm_trainingvia profiles/environment variable when conversion is neededThis ensures the host Docker daemon is only accessible when necessary.