Github user kpu commented on a diff in the pull request:

    https://github.com/apache/incubator-joshua/pull/68#discussion_r80882244
  
    --- Diff: jni/kenlm_wrap.cc ---
    @@ -76,15 +76,16 @@ class EqualIndex : public 
std::binary_function<StateIndex, StateIndex, bool> {
     typedef std::unordered_set<StateIndex, HashIndex, EqualIndex> Lookup;
     
     /**
    - * A Chart bundles together a unordered_multimap that maps ChartState 
signatures to a single
    - * object instantiated using a pool. This allows duplicate states to avoid 
allocating separate
    - * state objects at multiple places throughout a sentence, and also allows 
state to be shared
    - * across KenLMs for the same sentence.  Multimap is used to avoid hash 
collisions which can
    - * return incorrect results, and cause out-of-bounds lookups when multiple 
KenLMs are in use.
    + * A Chart bundles together a vector holding CharStates and an 
unordered_set of StateIndexes
    + * which provides a mapping between StateIndexes and the positions of 
ChartStates in the vector.
    + * This allows for duplicate states to avoid allocating separate state 
objects at multiple places
    + * throughout a sentence.
      */
     class Chart {
       public:
    -    Chart() : lookup_(1000, HashIndex(vec_), EqualIndex(vec_)) {}
    +    Chart(long* ngramBuffer) : 
    --- End diff --
    
    Good practice to use explicit with single-argument constructors.  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to