[GitHub] incubator-joshua issue #65: This PR is a first attempt to minimize ngram arr...

2016-09-23 Thread kpu
Github user kpu commented on the issue:

https://github.com/apache/incubator-joshua/pull/65
  
For estimateRuleCost, are we allocating a ChartState on a vector then 
throwing it away to score grammar rules in isolation?  I thought that was a 
separate JNI path that didn't want a sentence.  


---
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.
---


[GitHub] incubator-joshua issue #65: This PR is a first attempt to minimize ngram arr...

2016-09-21 Thread KellenSunderland
Github user KellenSunderland commented on the issue:

https://github.com/apache/incubator-joshua/pull/65
  
Well in that case I can open a PR without the DirectBuffer changes 
included.  They're pretty complicated and if they don't improve performance 
it's probably better to leave them out.


---
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.
---


[GitHub] incubator-joshua issue #65: This PR is a first attempt to minimize ngram arr...

2016-09-20 Thread mjpost
Github user mjpost commented on the issue:

https://github.com/apache/incubator-joshua/pull/65
  
I did some timing tests on commit 3ba83f84e8258a784fcef509fc9b158d44f15c66, 
and didn't see any change.


---
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.
---


[GitHub] incubator-joshua issue #65: This PR is a first attempt to minimize ngram arr...

2016-09-20 Thread KellenSunderland
Github user KellenSunderland commented on the issue:

https://github.com/apache/incubator-joshua/pull/65
  
@kpu I partially agree about sentence.  The sentence object is meant to be 
scoped to a single translation of a sentence.  For calls to probRule this helps 
disambiguate. 

For estimate I'm not so sure.  A rule can have estimateRuleCost called on 
it even when Joshua is not translation anything. For example as a result of 
calling sort on a grammar during startup before any requests are processed.  In 
fact the only usage of estimate being called perviously was passing null as the 
argument for sentence.  For the time being I'm creating the ChartState object 
once per call, and then immediately releasing it afterwards.  We'll have to 
test the performance impact of doing this, but this shouldn't actually be 
called when decoding so hopefully it won't affect decoding perf.


---
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.
---