Skip to content
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

Fix TRT selection in dpir #116

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

sgt0
Copy link
Contributor

@sgt0 sgt0 commented Apr 8, 2024

The cuda is None check is useless because cuda is always defined by line 73 after the 0b3a105 refactor (this is caught by mypy). So what actually ends up happening is that the TRT backend is always chosen if it's supported by the current system, even if the user explicitly wants another backend.

Before:

dpir(clip)
# backend = Backend.TRT

dpir(clip, cuda=False)
# backend = Backend.TRT

dpir(clip, cuda=True)
# backend = Backend.TRT

dpir(clip, cuda="trt")
# backend = Backend.TRT

After:

dpir(clip)
# backend = Backend.TRT

dpir(clip, cuda=False)
# backend = Backend.NCNN_VK

dpir(clip, cuda=True)
# backend = Backend.ORT_CUDA

dpir(clip, cuda="trt")
# backend = Backend.TRT

sgt0 added 3 commits April 8, 2024 17:45
The `cuda is None` check is useless because `cuda` is always defined by
line 73 after the 0b3a105 refactor (this is caught by mypy). So what
actually ends up happening is that the TRT backend is always chosen if
it's supported by the current system, even if the user explicitly wants
another backend.

Before:

    dpir(clip)
    # backend = Backend.TRT

    dpir(clip, cuda=False)
    # backend = Backend.TRT

    dpir(clip, cuda=True)
    # backend = Backend.TRT

    dpir(clip, cuda="trt")
    # backend = Backend.TRT

After:

    dpir(clip)
    # backend = Backend.TRT

    dpir(clip, cuda=False)
    # backend = Backend.NCNN_VK

    dpir(clip, cuda=True)
    # backend = Backend.ORT_CUDA

    dpir(clip, cuda="trt")
    # backend = Backend.TRT
There is no step with an ID of "dependencies" so the mypy step was never
running. We could add the ID to the "Install dependencies" step but
that's really not necessary when the job will fail early at that step
anyways if it doesn't succeed.

Since reintroducing mypy here will probably produce a bunch of errors
I'm sticking `continue-on-error: true` on it for now.
@Vodes
Copy link

Vodes commented Apr 8, 2024

Bruh

@Setsugennoao Setsugennoao merged commit 48ae3af into Jaded-Encoding-Thaumaturgy:master Apr 10, 2024
1 check passed
@sgt0 sgt0 deleted the dpir-trt branch April 10, 2024 20:09
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