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

Criteria.builder() confusing behaviour related to ordering #1961

Closed
Lundez opened this issue Aug 25, 2022 · 1 comment · Fixed by #1964
Closed

Criteria.builder() confusing behaviour related to ordering #1961

Lundez opened this issue Aug 25, 2022 · 1 comment · Fixed by #1964
Labels
bug Something isn't working

Comments

@Lundez
Copy link
Contributor

Lundez commented Aug 25, 2022

Description

If I call .optModelPath before .setTypes my optModelPath is removed, while if I call .setTypes first the optModelPath is still "active".

This bug caused me having an incredible amount of confusion!

Expected Behavior

Shouldn't matter which order we apply building steps.

Error Message

Model not found, or even worse when I tried to also remove Application that it loads ANOTHER model from a DIFFERENT path. That was incredibly confusing too, now my model required the wrong input rank!

How to Reproduce?

return Criteria.builder()
            .optEngine(engine.djlEngineName())
            .optDevice(device)
            .setTypes(String::class.java, String::class.java)
            .optModelPath(file.path)
            .optTranslator(translator)
            .optApplication(Application.NLP.SENTIMENT_ANALYSIS)
            .optProgress(ProgressBar())
            .build()

vs

return Criteria.builder()
            .optEngine(engine.djlEngineName())
            .optDevice(device)
            .optModelPath(file.path)
            .setTypes(String::class.java, String::class.java)
            .optTranslator(translator)
            .optApplication(Application.NLP.SENTIMENT_ANALYSIS)
            .optProgress(ProgressBar())
            .build()

Steps to reproduce

  1. See code update, I think the issue is attached to return new Builder<>(inputClass, outputClass, this);

Environment Info

Windows 11, WSL Ubuntu

FAILS
@Lundez Lundez added the bug Something isn't working label Aug 25, 2022
@frankfliu
Copy link
Contributor

@Lundez
Indeed, this is a bug! Thanks a lot for reporting is.

frankfliu added a commit to frankfliu/djl that referenced this issue Aug 25, 2022
Fixes deepjavalibrary#1961

Change-Id: Ic3b1ca69ddbfffc030a9f301ca129d4ee7fedd2b
KexinFeng pushed a commit that referenced this issue Aug 25, 2022
Fixes #1961

Change-Id: Ic3b1ca69ddbfffc030a9f301ca129d4ee7fedd2b
patins1 pushed a commit to patins1/djl that referenced this issue Aug 26, 2022
Fixes deepjavalibrary#1961

Change-Id: Ic3b1ca69ddbfffc030a9f301ca129d4ee7fedd2b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants