[ 
https://issues.apache.org/jira/browse/OPENNLP-1589?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17864349#comment-17864349
 ] 

ASF GitHub Bot commented on OPENNLP-1589:
-----------------------------------------

rzo1 commented on PR #635:
URL: https://github.com/apache/opennlp/pull/635#issuecomment-2218458211

   I am going to close this PR for now. Given the fact, that the f-measure on 
`main` was created as a baseline without using an actual cache, this PR should 
create the same results with caching.
   
   However, this is not the case, which implies, that the changes are not valid.
   
   However, there are still some open questions to better understand what 
actually happens here and how we can implement a more efficient way of caching 
something without changing the f-measure in the eval test.
   
   The following questions remain:
   
   - Why did we originally choose to use `==` instead of `Arrays.equals(...)` 
for caching the features.
   - Why does using `Arrays.equals(...)` only impact the eval tests for Dutch. 
   
   Side-note: Played around with `IdentityHashMap` to get a similar behaviour 
as on `main`. This will make the eval tests pass (as expected) because the 
cache numbers are identical, but doesn't make sense to impl it like that 
(because no gain)




> Fix incorrect array check in CachedFeatureGenerator
> ---------------------------------------------------
>
>                 Key: OPENNLP-1589
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-1589
>             Project: OpenNLP
>          Issue Type: Bug
>          Components: Name Finder
>    Affects Versions: 2.3.3
>            Reporter: Martin Wiesner
>            Assignee: Richard Zowalla
>            Priority: Major
>             Fix For: 2.4.0
>
>
> There is an invalid comparison for equality of two arrays in 
> CachedFeatureGenerator#createFeatures(..) in line 58.
> Currently, many situations exists in which no improvements by the caching are 
> achieved (as intended).
> This should be repaired and a test shall show the cache mechanism is working 
> correctly.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to