fixed a bunch of stuff #152
Conversation
…dded better normilzation, improved the sentence splitting, fixed some formatting
docker/gpu/Dockerfile
Outdated
| uv sync --extra gpu | ||
|
|
||
| # Copy project files including models and sync again | ||
| COPY --chown=appuser:appuser api ./api | ||
| COPY --chown=appuser:appuser web ./web | ||
| COPY --chown=appuser:appuser docker/scripts/ ./ | ||
| RUN chmod +x ./entrypoint.sh | ||
| RUN sed -i 's/\r$//' ./entrypoint.sh |
There was a problem hiding this comment.
Comment edited by @remsky to keep it productive and focused on the code
This issue happens due to your Git config on Windows adding Windows line endings to checked-out files. Setting core.autocrlf to input prevents this, eliminating the need for a workaround.
There was a problem hiding this comment.
I didn’t know that thank you for informing me I’ll look into it and remove the workaround.
There was a problem hiding this comment.
It works though @remsky u might wanna add a .gitatributes (I think that's what it called) so everyone has that auto set
docker/gpu/Dockerfile
Outdated
| @@ -32,14 +35,15 @@ COPY --chown=appuser:appuser pyproject.toml ./pyproject.toml | |||
|
|
|||
| # Install dependencies with GPU extras (using cache mounts) | |||
| RUN --mount=type=cache,target=/root/.cache/uv \ | |||
| uv venv && \ | |||
| uv venv --python 3.11 && \ | |||
There was a problem hiding this comment.
Why would you want to set it to a specific version if it's not a hard requirement?
There was a problem hiding this comment.
Believe its part of investigating the best solution to the espeak dependency bugs, which has involved a few version bumps and comparisons on the image etc to narrow in on.
Appreciate you taking the time to review @fedot, open source is a team effort, and every bit helps that encourages to that end.
There was a problem hiding this comment.
Why would you want to set it to a specific version if it's not a hard requirement?
Mostly because it was built and tested with Python 3.10 (I changed it in the latest version so its the correct version). The idea is that even if a package doesn't have a version for the latest version of python it will still be set to python 3.10 inside the container
| "Ω":"ohm", "kΩ":"kiloohm", "mΩ":"megaohm", # Resistance (Ohm) | ||
| "f":"farad", "µf":"microfarad", "nf":"nanofarad", "pf":"picofarad", # Capacitance | ||
| "b":"byte", "kb":"kilobyte", "mb":"megabyte", "gb":"gigabyte", "tb":"terabyte", "pb":"petabyte", # Data size | ||
| "kbps":"kilobyte per second","mbps":"megabyte per second","gbps":"gigabyte per second", |
There was a problem hiding this comment.
Comment edited by @remsky to retain constructive elements
- By convention: kb, mb, gb, etc., typically refer to bits, not bytes.
- Similarly: kbps (or kb/s) means kilobits per second, while kBps (or kB/s) refers to kilobytes per second
The capitalization of the B shows the distinction
There was a problem hiding this comment.
I do agree the main issue I faced though is that some are capitalization independent while others are not but I do have some ideas so I will also look into that too. Thanks for the feedback.
|
Folks, thank you both for the time and effort! 👍❤️ |
Made the api use the normalizer, fixed the docker compose not starting on windows because of line endings, fixed the wrong version of espeak, added better normilzation, improved the sentence splitting, fixed some formatting