Skip to content

Added a single test function script and fix debug-test.sh to be more robust#7279

Merged
mofosyne merged 6 commits intoggml-org:masterfrom
mofosyne:run-single-test-script
May 17, 2024
Merged

Added a single test function script and fix debug-test.sh to be more robust#7279
mofosyne merged 6 commits intoggml-org:masterfrom
mofosyne:run-single-test-script

Conversation

@mofosyne
Copy link
Copy Markdown
Collaborator

@mofosyne mofosyne commented May 14, 2024

Continuation of #7096 somewhat to address cases where a person may just want to run a single test.

Also discovered that the test binaries expects to be run in the git root context so need to fix debug-test.sh to take that into account.

@josh-ramer you may want to have a quick look a this to see if this would also be useful to you as well.

This is what the output visually looks like. With emphasis on the last three lines for clearer feedback

image


(Created this to try and more quickly pin down a CI issue)

@mofosyne
Copy link
Copy Markdown
Collaborator Author

Actually @josh-ramer when I ran ./scripts/debug-test.sh test 24 with this script I noticed this printout

Ran Test #24: test-eval-callback
GDB args: /home/mofosyne/gitextern/llama.cpp/build-ci-debug/bin/eval-callback "--hf-repo" "ggml-org/models" "--hf-file" "tinyllamas/stories260K.gguf" "--model" "stories260K.gguf" "--prompt" "hello" "--seed" "42" "-ngl" "0"
GDB args @: /home/mofosyne/gitextern/llama.cpp/build-ci-debug/bin/eval-callback~/gitextern/llama.cpp ~/gitextern/llama.cpp/build-ci-debug ~/gitextern/llama.cpp ~/gitextern/llama.cpp

Am i reading it correctly? I would have thought that args[@] would be the same as gdb_args[test_number] but with path expanded, but that doesn't seem to be the case.

@mofosyne mofosyne added script Script related Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix labels May 14, 2024
@josh-ramer
Copy link
Copy Markdown
Contributor

josh-ramer commented May 14, 2024

That comment is a little deceptive, what the code really does is find paths with /../ in them & replace the path/../path with ../path. There is probably a better solution than that out there if you're pretty skilled in bash scripting. Maybe one easy alternative would be to remove the suffix before /../.

Nice addition, makes sense to be able to execute as well.

@mofosyne
Copy link
Copy Markdown
Collaborator Author

Maybe you can suggest a better comment? Either way, see if this script is working well for your flow and if I didn't break your gdb flow, then well should be good to go!

@josh-ramer
Copy link
Copy Markdown
Contributor

I've got a review started, but there are no EC2 spot instance available right now (first time I've had that happen to my workflow) so I haven't run it yet to make sure it works, but as soon as they're available, I plan to run it for you.

Copy link
Copy Markdown
Contributor

@josh-ramer josh-ramer left a comment

Choose a reason for hiding this comment

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

LGTM, will run as soon as I have the compute available.

Comment thread scripts/debug-test.sh Outdated
@mofosyne mofosyne self-assigned this May 15, 2024
@mofosyne mofosyne marked this pull request as draft May 15, 2024 01:49
@mofosyne mofosyne marked this pull request as ready for review May 15, 2024 23:25
@mofosyne mofosyne added merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. and removed merge ready A maintainer can use this label to indicate that they consider the changes final and ready to merge. labels May 15, 2024
@mofosyne
Copy link
Copy Markdown
Collaborator Author

@josh-ramer ready

@josh-ramer
Copy link
Copy Markdown
Contributor

LGTM

@mofosyne
Copy link
Copy Markdown
Collaborator Author

@josh-ramer see if you can press approve as there needs to be one other approval before it can merge in.

Copy link
Copy Markdown
Contributor

@josh-ramer josh-ramer left a comment

Choose a reason for hiding this comment

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

LGTM

@josh-ramer
Copy link
Copy Markdown
Contributor

@josh-ramer see if you can press approve as there needs to be one other approval before it can merge in.

Looks like I don't have write access. I think that means I'd have to be added as a collaborator like you.

@mofosyne mofosyne merged commit 51e9d02 into ggml-org:master May 17, 2024
@mofosyne mofosyne deleted the run-single-test-script branch May 17, 2024 12:40
@mofosyne
Copy link
Copy Markdown
Collaborator Author

@josh-ramer thanks. It's in now, hope it helps!

Seunghhon pushed a commit to Seunghhon/llama.cpp that referenced this pull request Apr 26, 2026
…robust (ggml-org#7279)

* run-single-test.sh: added a single test function script and fix debug-test.sh to be more robust

* debug-test.sh: combined execute and gdb test mode via -g flag

* debug-test.sh: refactor

* debug-test: refactor for clarity

* debug-test.sh: comment style changes

* debug-test.sh: fix gdb
phuongncn pushed a commit to phuongncn/llama.cpp-gx10-dgx-sparks-deepseekv4 that referenced this pull request Apr 28, 2026
…robust (ggml-org#7279)

* run-single-test.sh: added a single test function script and fix debug-test.sh to be more robust

* debug-test.sh: combined execute and gdb test mode via -g flag

* debug-test.sh: refactor

* debug-test: refactor for clarity

* debug-test.sh: comment style changes

* debug-test.sh: fix gdb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Review Complexity : Low Trivial changes to code that most beginner devs (or those who want a break) can tackle. e.g. UI fix script Script related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants