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

RuntimeException: inferenceWithScores not supported for ViterbiLayer #655

Open
hriaz17 opened this issue Jul 15, 2022 · 7 comments
Open

Comments

@hriaz17
Copy link

hriaz17 commented Jul 15, 2022

I am trying to get the scores for the predictions of the LSTM-CRF model.

In line 327 of Metal.scala, replacing: Layers.predict(model, taskId, sentence, modHeadPairsOpt, constEmbeddings) function with Layers.predictWithScores(model, taskId, sentence, modHeadPairsOpt, constEmbeddings, true) causes the exception to be raised.

In line 295 of Layers.scala, predictWithScores makes a call to inferenceWithScores in ViterbiForwardLayer.scala which is not implemented.

Should I replace the call to inferenceWithScores with just inference, which is being used by the default Layers.predict?

@kwalcock
Copy link
Member

I'm finally looking. Sorry for the delay.

@kwalcock
Copy link
Member

I wonder whether the scores are really necessary given that access isn't provided. So the hierarchy is

FinalLayer
  <- ForwardLayer
    <- GreedyForwardLayer
    <- ViterbiForwardLayer

and Greedy has it while Viterbi doesn't. It seems like there needs to be a Utils.viterbiWithScores for ViterbiForwardLayer.inferenceWithScores in the same way inference calls Utils.viterbi. That could probably be made by keeping track of backScores along with backPointers.

That probably doesn't answer your question. If you replace inferenceWithScore with just inference won't you lose the scores you are trying to locate? Is it loss() that you need instead?

@MihaiSurdeanu
Copy link
Contributor

This one is on me. I implemented it for greedy inference, and did not implement it yet for Viterbi.
Haris, for now please change the inference type to greedy in the final layer, in the config file. It should work afterwards.

@hriaz17
Copy link
Author

hriaz17 commented Jul 15, 2022

I guess what I really need to return from the viterbi() method in Utils.scala is pathScore along with bestPath. That would involve minor modifications to viterbi() and inference() but this is a short term solution that won't return an IndexedSeq[IndexedSeq[(String, Float)] nested array as specified in the inferenceWithScores() method.

@hriaz17
Copy link
Author

hriaz17 commented Jul 15, 2022

This one is on me. I implemented it for greedy inference, and did not implement it yet for Viterbi. Haris, for now please change the inference type to greedy in the final layer, in the config file. It should work afterwards.

ok.

@kwalcock
Copy link
Member

@harisriaz17, keep in mind there is one viterbi in org.clulab.dynet.Utils. Perhaps in the end one version can be used for both the existing and the new case.

@kwalcock
Copy link
Member

Will this for sure be needed eventually, or is it contingent on things that might not happen? I.e., will implementing it now be potentially wasted effort?

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

No branches or pull requests

3 participants