propagation of feature name+debug from factory, return correct features array for fsa ffs

git-svn-id: https://ws10smt.googlecode.com/svn/trunk@486 ec762483-ff6d-05da-a07a-a48fb63a330f

14 files changed, 145 insertions, 36 deletions

diff --git a/decoder/cdec_ff.cc b/decoder/cdec_ff.cc
index 6edac126..1ef50eb1 100644
--- a/decoder/cdec_ff.cc
+++ b/decoder/cdec_ff.cc
@@ -13,8 +13,12 @@
 #include "ff_register.h"
 
 void register_feature_functions() {
+  RegisterFsaImpl<SameFirstLetter>(true,false);
+  RegisterFsaImpl<LongerThanPrev>(true,false);
   RegisterFF<LanguageModel>();
   RegisterFsaImpl<LanguageModelFsa>(true,false); // same as LM but using fsa wrapper
+  ff_registry.Register("LanguageModelFsaDynamic",new FFFactory<FeatureFunctionFromFsa<FsaFeatureFunctionDynamic<LanguageModelFsa> > >); // test correctness of FsaFeatureFunctionDynamic erasure
+  RegisterFsaDynToFF<SameFirstLetter>();
 
   RegisterFF<WordPenalty>();
   RegisterFF<SourceWordPenalty>();
@@ -23,8 +27,6 @@ void register_feature_functions() {
 
   //TODO: worthless example target FSA ffs.  remove later
   ff_registry.Register(new FFFactory<WordPenaltyFromFsa>); // same as WordPenalty, but implemented using ff_fsa
-  RegisterFsaImpl<SameFirstLetter>(true,false);
-  ff_registry.Register(new FFFactory<FeatureFunctionFromFsa<LongerThanPrev> >);
   ff_registry.Register(new FFFactory<FeatureFunctionFromFsa<ShorterThanPrev> >);
 
   //TODO: use for all features the new Register which requires static FF::usage(false,false) give name
diff --git a/decoder/fdict.cc b/decoder/fdict.cc
index 65187685..baa0b552 100644
--- a/decoder/fdict.cc
+++ b/decoder/fdict.cc
@@ -2,12 +2,26 @@
 #include "stdlib.h"
 //for malloc (need on cygwin); todo <cstdlib> and std::malloc
 #include <string>
+#include <sstream>
 
 using namespace std;
 
 Dict FD::dict_;
 bool FD::frozen_ = false;
 
+std::string FD::Convert(std::vector<WordID> const& v) {
+    return Convert(&*v.begin(),&*v.end());
+}
+
+std::string FD::Convert(WordID const *b,WordID const* e) {
+  ostringstream o;
+  for (WordID const* i=b;i<e;++i) {
+    if (i>b) o << ' ';
+    o << FD::Convert(*i);
+  }
+  return o.str();
+}
+
 static int HexPairValue(const char * code) {
   int value = 0;
   const char * pch = code;
diff --git a/decoder/fdict.h b/decoder/fdict.h
index c4236580..f9673023 100644
--- a/decoder/fdict.h
+++ b/decoder/fdict.h
@@ -20,6 +20,9 @@ struct FD {
   static inline const std::string& Convert(const WordID& w) {
     return dict_.Convert(w);
   }
+  static std::string Convert(WordID const *i,WordID const* e);
+  static std::string Convert(std::vector<WordID> const& v);
+
   // Escape any string to a form that can be used as the name
   // of a weight in a weights file
   static std::string Escape(const std::string& s);
diff --git a/decoder/ff.cc b/decoder/ff.cc
index a23f1655..5c52ca2b 100644
--- a/decoder/ff.cc
+++ b/decoder/ff.cc
@@ -46,7 +46,7 @@ Features ModelSet::all_features(std::ostream *warn,bool warn0) {
     string const& ffname=ff.name_;
     FFS si=ff.features();
     if (si.empty()) {
-      WARNFF(ffname<<" doesn't yet report any feature IDs - implement features() method?");
+      WARNFF(ffname<<" doesn't yet report any feature IDs - either supply feature weight, or use --no_freeze_feature_set, or implement features() method");
     }
     unsigned n0=0;
     for (unsigned j=0;j<si.size();++j) {
diff --git a/decoder/ff.h b/decoder/ff.h
index 2de3c1fb..e3bfe392 100644
--- a/decoder/ff.h
+++ b/decoder/ff.h
@@ -1,6 +1,14 @@
 #ifndef _FF_H_
 #define _FF_H_
 
+#define DEBUG_INIT 0
+#if DEBUG_INIT
+# include <iostream>
+# define DBGINIT(a) do { std::cerr<<a<<"\n"; } while(0)
+#else
+# define DBGINIT(a)
+#endif
+
 #include <vector>
 #include <cstring>
 #include "fdict.h"
@@ -19,6 +27,12 @@ class FeatureFunction {
  public:
   std::string name_; // set by FF factory using usage()
   bool debug_; // also set by FF factory checking param for immediate initial "debug"
+  //called after constructor, but before name_ and debug_ have been set
+  virtual void Init() { DBGINIT("default FF::Init name="<<name_); }
+  virtual void init_name_debug(std::string const& n,bool debug) {
+    name_=n;
+    debug_=debug;
+  }
   bool debug() const { return debug_; }
   FeatureFunction() : state_size_() {}
   explicit FeatureFunction(int state_size) : state_size_(state_size) {}
@@ -36,7 +50,7 @@ public:
   virtual bool rule_feature() const { return false; }
 
   //OVERRIDE THIS:
-  virtual Features features() const { return Features(); }
+  virtual Features features() const { return single_feature(FD::Convert(name_)); }
   // returns the number of bytes of context that this feature function will
   // (maximally) use.  By default, 0 ("stateless" models in Hiero/Joshua).
   // NOTE: this value is fixed for the instance of your class, you cannot
@@ -63,6 +77,7 @@ public:
   // if there's some state left when you transition to the goal state, score
   // it here.  For example, the language model computes the cost of adding
   // <s> and </s>.
+
 protected:
   virtual void FinalTraversalFeatures(const void* residual_state,
                                       FeatureVector* final_features) const;
diff --git a/decoder/ff_factory.h b/decoder/ff_factory.h
index 102e709c..5eb68c8b 100644
--- a/decoder/ff_factory.h
+++ b/decoder/ff_factory.h
@@ -42,7 +42,9 @@ struct FactoryBase : public UntypedFactory {
 template<class FF>
 struct FFFactory : public FactoryBase<FeatureFunction> {
   FP Create(std::string param) const {
-    return FP(new FF(param));
+    FF *ret=new FF(param);
+    ret->Init();
+    return FP(ret);
   }
   virtual std::string usage(bool params,bool verbose) const {
     return FF::usage(params,verbose);
@@ -54,7 +56,9 @@ struct FFFactory : public FactoryBase<FeatureFunction> {
 template<class FF>
 struct FsaFactory : public FactoryBase<FsaFeatureFunction> {
   FP Create(std::string param) const {
-    return FP(new FF(param));
+    FF *ret=new FF(param);
+    ret->Init();
+    return FP(ret);
   }
   virtual std::string usage(bool params,bool verbose) const {
     return FF::usage(params,verbose);
@@ -94,8 +98,8 @@ struct FactoryRegistry : public UntypedFactoryRegistry {
     if (debug)
       cerr<<"debug enabled for "<<ffname<< " - remaining options: '"<<param<<"'\n";
     FP res = dynamic_cast<FB const&>(*it->second).Create(param);
-    res->name_ = ffname;
-    res->debug_ = debug;
+    res->init_name_debug(ffname,debug);
+    // could add a res->Init() here instead of in Create if we wanted feature id to potentially differ based on the registered name rather than static usage() - of course, specific feature ids can be computed on the basis of feature param as well; this only affects the default single feature id=name
     return res;
   }
 };
diff --git a/decoder/ff_from_fsa.h b/decoder/ff_from_fsa.h
index 2c339aa8..ec1a28fa 100755
--- a/decoder/ff_from_fsa.h
+++ b/decoder/ff_from_fsa.h
@@ -30,14 +30,21 @@ class FeatureFunctionFromFsa : public FeatureFunction {
 public:
   FeatureFunctionFromFsa(std::string const& param) : ff(param) {
     debug_=true; // because factory won't set until after we construct.
-    Init();
   }
 
   static std::string usage(bool args,bool verbose) {
     return Impl::usage(args,verbose);
   }
+  void init_name_debug(std::string const& n,bool debug) {
+    FeatureFunction::init_name_debug(n,debug);
+    ff.init_name_debug(n,debug);
+  }
 
-  Features features() const { return ff.features(); }
+  // this should override
+  Features features() const {
+    DBGINIT("FeatureFunctionFromFsa features() name="<<ff.name()<<" features="<<FD::Convert(ff.features()));
+    return ff.features();
+  }
 
   // Log because it potentially stores info in edge.  otherwise the same as regular TraversalFeatures.
   void TraversalFeaturesLog(const SentenceMetadata& smeta,
@@ -227,16 +234,20 @@ public:
     assert(left_end(w2,w2+2)==w2+1);
   }
 
-private:
-  Impl ff;
+  // override from FeatureFunction; should be called by factory after constructor.
   void Init() {
-//    FeatureFunction::name=Impl::usage(false,false); // already achieved by ff_factory.cc
+    ff.Init();
+    ff.sync();
+    DBGINIT("base (single feature) FsaFeatureFunctionBase::Init name="<<name_<<" features="<<FD::Convert(features()));
+//    FeatureFunction::name_=Impl::usage(false,false); // already achieved by ff_factory.cc
     M=ff.markov_order();
     ssz=ff.state_bytes();
     state_offset=sizeof(WordID)*M;
     SetStateSize(ssz+state_offset);
     assert(!ssz == !M); // no fsa state <=> markov order 0
   }
+private:
+  Impl ff;
   int M; // markov order (ctx len)
   FeatureFunctionFromFsa(); // not allowed.
 
diff --git a/decoder/ff_fsa.h b/decoder/ff_fsa.h
index e7877dd5..6416151c 100755
--- a/decoder/ff_fsa.h
+++ b/decoder/ff_fsa.h
@@ -68,18 +68,26 @@ usage: see ff_sample_fsa.h or ff_lm_fsa.h
 
 template <class Impl>
 struct FsaFeatureFunctionBase : public FsaFeatureFunctionData {
-  // CALL 1 of these MANUALLY  (because feature name(s) may depend on param, it's not done in ctor)
-  void Init(std::string const& fname="") {
-    fid_=FD::Convert(fname.empty()?name():fname);
+  Impl const& d() const { return static_cast<Impl const&>(*this); }
+  Impl & d()  { return static_cast<Impl &>(*this); }
+
+  // this will get called by factory - override if you have multiple or dynamically named features.  note: may be called repeatedly
+  void Init() {
+    Init(name());
+    DBGINIT("base (single feature) FsaFeatureFunctionBase::Init name="<<name()<<" features="<<FD::Convert(features_));
+  }
+  void Init(std::string const& fname) {
+    fid_=FD::Convert(fname);
     InitHaveFid();
   }
-  Features features_;
   void InitHaveFid() {
     features_=FeatureFunction::single_feature(fid_);
   }
+  Features features() const {
+    DBGINIT("FeatureFunctionBase::features() name="<<name()<<" features="<<FD::Convert(features_));
+    return features_;
+  }
 
-  Impl const& d() const { return static_cast<Impl const&>(*this); }
-  Impl & d()  { return static_cast<Impl &>(*this); }
 public:
   int fid_; // you can have more than 1 feature of course.
 
@@ -252,8 +260,11 @@ public:
   }
 
   // don't set state-bytes etc. in ctor because it may depend on parsing param string
-  FsaFeatureFunctionBase(int statesz=0,Sentence const& end_sentence_phrase=Sentence()) :
-    FsaFeatureFunctionData(statesz,end_sentence_phrase) {  }
+  FsaFeatureFunctionBase(int statesz=0,Sentence const& end_sentence_phrase=Sentence())
+    : FsaFeatureFunctionData(statesz,end_sentence_phrase)
+  {
+    name_=name(); // should allow FsaDynamic wrapper to get name copied to it with sync
+  }
 
 };
 
diff --git a/decoder/ff_fsa_data.h b/decoder/ff_fsa_data.h
index 3252c5ac..2f0c55b8 100755
--- a/decoder/ff_fsa_data.h
+++ b/decoder/ff_fsa_data.h
@@ -6,17 +6,35 @@
 #include "sentences.h"
 #include "feature_accum.h"
 #include "value_array.h"
-
+#include "ff.h" //debug
 typedef ValueArray<uint8_t> Bytes;
 
-// stuff I see no reason to have virtual.  but there's a diamond inheritance problem to solve now when type erasing the CRTP impl wrapper.  virtual inheritance would slow things?
+// stuff I see no reason to have virtual.  but because it's impossible (w/o virtual inheritance to have dynamic fsa ff know where the impl's data starts, implemented a sync (copy) method that needs to be called.  init_name_debug was already necessary to keep state in sync between ff and ff_from_fsa, so no sync should be needed after it.  supposing all modifications were through setters, then no explicit sync call would ever be needed; updates could be mirrored.
 struct FsaFeatureFunctionData
 {
+  void init_name_debug(std::string const& n,bool debug) {
+    name_=n;
+    debug_=debug;
+  }
   //HACK for diamond inheritance (w/o costing performance)
   FsaFeatureFunctionData *sync_to_;
 
   void sync() const { // call this if you modify any fields after your constructor is done
-    if (sync_to_) *sync_to_=*this;
+
+    if (sync_to_) {
+      DBGINIT("sync to "<<*sync_to_);
+      *sync_to_=*this;
+      DBGINIT("synced result="<<*sync_to_<< " from this="<<*this);
+    } else {
+      DBGINIT("nobody to sync to - from FeatureFunctionData this="<<*this);
+    }
+  }
+
+  friend std::ostream &operator<<(std::ostream &o,FsaFeatureFunctionData const& d) {
+    o << "[FSA "<<d.name_<<" features="<<FD::Convert(d.features_)<<" start=";
+    d.print_state(o,d.start_state());
+    o<<"]";
+    return o;
   }
 
   FsaFeatureFunctionData(int statesz=0,Sentence const& end_sentence_phrase=Sentence()) : ssz(statesz),start(statesz),h_start(statesz),end_phrase_(end_sentence_phrase) {
@@ -68,8 +86,8 @@ struct FsaFeatureFunctionData
       print_hex_byte(o,*i);
   }
 
-protected:
   Features features_;
+protected:
   int ssz; // don't forget to set this. default 0 (it may depend on params of course)
   Bytes start,h_start; // start state and estimated-features (heuristic) start state.  set these.  default empty.
   Sentence end_phrase_; // words appended for final traversal (final state cost is assessed using Scan) e.g. "</s>" for lm.
diff --git a/decoder/ff_fsa_dynamic.h b/decoder/ff_fsa_dynamic.h
index 2a26676d..d03fddee 100755
--- a/decoder/ff_fsa_dynamic.h
+++ b/decoder/ff_fsa_dynamic.h
@@ -26,10 +26,14 @@ struct FsaFeatureFunction : public FsaFeatureFunctionData {
   virtual void *ScanPhraseAccumBounce(SentenceMetadata const& smeta,Hypergraph::Edge const& edge,WordID const* i, WordID const* end,void *cs,void *ns,Accum *accum) const = 0;
 
   virtual int early_score_words(SentenceMetadata const& smeta,Hypergraph::Edge const& edge,WordID const* i, WordID const* end,Accum *accum) const { return 0; }
+  // called after constructor, before use
+  virtual void Init() = 0;
   virtual std::string usage_v(bool param,bool verbose) const {
     return FeatureFunction::usage_helper("unnamed_dynamic_fsa_feature","","",param,verbose);
   }
-
+  virtual void init_name_debug(std::string const& n,bool debug) {
+    FsaFeatureFunctionData::init_name_debug(n,debug);
+  }
 
   virtual void print_state(std::ostream &o,void const*state) const {
     FsaFeatureFunctionData::print_state(o,state);
@@ -49,10 +53,13 @@ struct FsaFeatureFunction : public FsaFeatureFunctionData {
 // you might be wondering: why do this?  answer: it's cool, and it means that the bottom-up ff over ff_fsa wrapper doesn't go through multiple layers of dynamic dispatch
 // usage: typedef FsaFeatureFunctionDynamic<MyFsa> MyFsaDyn;
 template <class Impl>
-struct FsaFeatureFunctionDynamic : public FsaFeatureFunction,Impl {
+struct FsaFeatureFunctionDynamic : public FsaFeatureFunction {
   static const bool simple_phrase_score=Impl::simple_phrase_score;
-  Impl& d() { return static_cast<Impl&>(*this); }
-  Impl const& d() const { return static_cast<Impl const&>(*this); }
+  Impl& d() { return impl;//static_cast<Impl&>(*this);
+  }
+  Impl const& d() const { return impl;
+    //static_cast<Impl const&>(*this);
+  }
   int markov_order() const { return d().markov_order(); }
 
   virtual void ScanAccum(SentenceMetadata const& smeta,Hypergraph::Edge const& edge,
@@ -92,11 +99,23 @@ struct FsaFeatureFunctionDynamic : public FsaFeatureFunction,Impl {
     return d().print_state(o,state);
   }
 
-  FsaFeatureFunctionDynamic(std::string const& param) : Impl(param) {
+  void init_name_debug(std::string const& n,bool debug) {
+    FsaFeatureFunction::init_name_debug(n,debug);
+    d().init_name_debug(n,debug);
+  }
+
+  virtual void Init() {
     d().sync_to_=(FsaFeatureFunction*)this;
+    d().Init();
     d().sync();
   }
 
+  FsaFeatureFunctionDynamic(std::string const& param) : impl(param) {
+    Init();
+  }
+private:
+  Impl impl;
+
 };
 
 
diff --git a/decoder/ff_lm.cc b/decoder/ff_lm.cc
index 61d845d6..f3e65cb7 100644
--- a/decoder/ff_lm.cc
+++ b/decoder/ff_lm.cc
@@ -609,12 +609,13 @@ void LanguageModelFsa::set_ngram_order(int i) {
       //TODO: reevaluate whether state space comes cleared by allocator or not.
     }
   }
+  sync(); // for dynamic markov_order copy etc
 }
 
 LanguageModelFsa::LanguageModelFsa(string const& param) {
   int lmorder;
   pimpl_ = make_lm_impl(param,&lmorder,&fid_);
-  InitHaveFid();
+  Init();
   floor_=pimpl_->floor_;
   set_ngram_order(lmorder);
 }
diff --git a/decoder/ff_lm_fsa.h b/decoder/ff_lm_fsa.h
index 1c8ebdad..d2df943e 100755
--- a/decoder/ff_lm_fsa.h
+++ b/decoder/ff_lm_fsa.h
@@ -115,8 +115,15 @@ struct LanguageModelFsa : public FsaFeatureFunctionBase<LanguageModelFsa> {
 
   // impl details:
   void set_ngram_order(int i); // if you build ff_from_fsa first, then increase this, you will get memory overflows.  otherwise, it's the same as a "-o i" argument to constructor
+  // note: if you adjust ngram_order, ff_from_fsa won't notice.
+
   double floor_; // log10prob minimum used (e.g. unk words)
 
+  // because we might have a custom fid due to lm name option:
+  void Init() {
+    InitHaveFid();
+  }
+
 private:
   int ngram_order_;
   int ctxlen_; // 1 less than above
diff --git a/decoder/ff_sample_fsa.h b/decoder/ff_sample_fsa.h
index 40a32bae..20d64b16 100755
--- a/decoder/ff_sample_fsa.h
+++ b/decoder/ff_sample_fsa.h
@@ -14,12 +14,14 @@ struct WordPenaltyFsa : public FsaFeatureFunctionBase<WordPenaltyFsa> {
   }
 
   WordPenaltyFsa(std::string const& param) {
-    Init();
+#if 0
+    Init(); // gets called by factory already
     return; //below are all defaults:
     set_state_bytes(0);
     start.clear();
     h_start.clear();
-  }
+#endif
+    }
   // move from state to next_state after seeing word x, while emitting features->add_value(fid,val) possibly with duplicates.  state and next_state may be same memory.
   Featval Scan1(WordID w,void const* state,void *next_state) const {
     return 1;
@@ -32,7 +34,8 @@ struct SameFirstLetter : public FsaFeatureFunctionBase<SameFirstLetter> {
   SameFirstLetter(std::string const& param) : FsaFeatureFunctionBase<SameFirstLetter>(1,singleton_sentence("END"))
                                                // 1 byte of state, scan final (single) symbol "END" to get final state cost
   {
-    start[0]='a'; h_start[0]=0; Init();
+    start[0]='a'; h_start[0]=0;
+//    Init();
   }
   int markov_order() const { return 1; }
   Featval Scan1(WordID w,void const* old_state,void *new_state) const {
@@ -82,6 +85,7 @@ struct LongerThanPrev : public FsaFeatureFunctionBase<LongerThanPrev> {
   }
   int markov_order() const { return 1; }
   LongerThanPrev(std::string const& param) : Base(sizeof(int)/* ,singleton_sentence(TD::se) */) {
+#if 0
     Init();
     if (0) { // all this is done in constructor already
       set_state_bytes(sizeof(int));
@@ -96,7 +100,7 @@ struct LongerThanPrev : public FsaFeatureFunctionBase<LongerThanPrev> {
     assert(h_start.size()==sizeof(int));
     state(start.begin())=999999;
     state(h_start.begin())=4; // estimate: anything >4 chars is usually longer than previous
-
+#endif
   }
 
   Featval Scan1(WordID w,void const* from,void *next_state) const {
diff --git a/decoder/perro.sh b/decoder/perro.sh
index 8df71855..836ad07f 100755
--- a/decoder/perro.sh
+++ b/decoder/perro.sh
@@ -1 +1 @@
-$gdb $cdec -k 30 --show_features -c fsa-hiero.ini -i perro.ur
+$gdb $cdec -k 30 --show_features -c fsa-hiero.ini -i perro.ur "$@"