clulab / processors

Natural Language Processors
https://clulab.github.io/processors/
Apache License 2.0
418 stars 101 forks source link

Print matchLookAhead and Behind in OdinStarter #796

Open kwalcock opened 5 months ago

kwalcock commented 5 months ago

@navalani, these lines might be missing.

kwalcock commented 5 months ago

@navalani. They might need to print out something more complete.

navalani commented 5 months ago

I see. I'll make those changes. By more complete, do you mean the token offsets at which they match?

kwalcock commented 5 months ago

@navalani, I was wrong. I see the "hello" come up in the next Inst.

kwalcock commented 5 months ago

@navalani, there is additional code to store more than one match per Inst.

kwalcock commented 5 months ago

@navalani, since Insts are identified only by the posId, they need to be compared in a different way. It was an optimization.

kwalcock commented 5 months ago

@navalani, I notice that visualization code has returned to OdinStarter. At one time it was moved to the Debugger. The duplication may be causing you or the computer some confusion.

navalani commented 5 months ago

Right. Let me work on transferring the code over.

On Thu, Apr 25, 2024, 8:12 AM Keith Alcock @.***> wrote:

External Email

@navalani https://github.com/navalani, I notice that visualization code has returned to OdinStarter. At one time it was moved to the Debugger. The duplication may be causing you or the computer some confusion.

— Reply to this email directly, view it on GitHub https://github.com/clulab/processors/pull/796#issuecomment-2077524409, or unsubscribe https://github.com/notifications/unsubscribe-auth/BCCR2VKGHSHIFR3TTUFE673Y7EML5AVCNFSM6AAAAABGX4KQOOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXGUZDINBQHE . You are receiving this because you were mentioned.Message ID: @.***>

navalani commented 5 months ago

visualize() and visualizeExtractor() are found both in the OdinStarter and in the Debugger, I am assuming we want them only in the Debugger and calls made to them from the Odin starter

kwalcock commented 5 months ago

At one time they were moved there. The code may have diverged, though, so caution is advised.

navalani commented 5 months ago

Got it. Thanks

kwalcock commented 5 months ago

@navalani, FWIW I don't see that the IdentityHashMap from the ThompsonVM made it into your code. I would guess that's why au is being overwritten by chocolat. It looks like all four As are matching. Git is difficult to figure out and I think some things are getting lost, some things that might have been helpful.

navalani commented 5 months ago

You're right, it'll be useful to run diff with the two versions of the Thompson VM. Let me work on that

navalani commented 5 months ago

@kwalcock is the IdentityHashMap in the ThompsonVM in the debugContext branch? I don't see much difference in the files

kwalcock commented 5 months ago

@navalani, it should be in this branch, kwalcock/odin-debugger, from which this PR originates.