I really don't think extension is the right way to go here. Domains don't
create feature vector variables, I'd much rather have a member.


On Tue, Nov 19, 2013 at 1:13 PM, David Belanger <notifications@github.com>wrote:

> In src/main/scala/cc/factorie/app/classify/backend/FeatureTemplates.scala:
>
> > @@ -0,0 +1,124 @@
> > +package cc.factorie.app.classify.backend
> > +
> > +import cc.factorie.variable._
> > +import cc.factorie.la.{DenseTensor1, WeightsMapAccumulator, DenseTensor2, Tensor1}
> > +import cc.factorie.model.{Weights2, Parameters}
> > +
> > +
> > +class FeatureTemplateVectorVariable(val template: FeatureTemplate[_], val domain: CategoricalVectorDomain[String])  extends FeatureVectorVariable[String]
> > +class HashedFeatureTemplateVectorVariable(val template: FeatureTemplate[_], val domain: DiscreteDomain)  extends cc.factorie.variable.
> HashFeatureVectorVariable
> > +
> > +//templates have no state. They do not have associated domains. You pass a template to a domain, and it returns a feature vector that points to this domain.
> > +trait FeatureTemplateDomain {
> > +  def domain: VectorDomain
>
> How about changing the these domains to not have a .domain member, but to
> extend an actual factorie domain? This will simplify syntax too. trait
> FeatureTemplateDomain { def size: Int def
> newBlankFeatureVectorVar(template: FeatureTemplate[_]):
> FeatureVectorVar[String] } class CategoricalFeatureTemplateDomain extends
> FeatureTemplateDomain with CategoricalVectorDomain[String]{ def size =
> this.dimensionSize def newBlankFeatureVectorVar(template:
> FeatureTemplate[_]) = new FeatureTemplateVectorVariable(template,this) }
> class HashedFeatureTemplateDomain(size: Int) extends DiscreteDomain(size)
> with FeatureTemplateDomain { def newBlankFeatureVectorVar(template:
> FeatureTemplate[_]) = new
> HashedFeatureTemplateVectorVariable(template,this) }
> … <#14271916d317ee01_>
> On Nov 19, 2013, at 12:46 PM, Andrew McCallum <notifications@github.com>
> wrote: In
> src/main/scala/cc/factorie/app/classify/backend/FeatureTemplates.scala: >
> @@ -0,0 +1,124 @@ > +package cc.factorie.app.classify.backend > + > +import
> cc.factorie.variable._ > +import cc.factorie.la.{DenseTensor1,
> WeightsMapAccumulator, DenseTensor2, Tensor1} > +import
> cc.factorie.model.{Weights2, Parameters} > + > + > +class
> FeatureTemplateVectorVariable(val template: FeatureTemplate[_], val domain:
> CategoricalVectorDomain[String]) extends FeatureVectorVariable[String] >
> +class HashedFeatureTemplateVectorVariable(val template:
> FeatureTemplate[_], val domain: DiscreteDomain) extends
> cc.factorie.variable.HashFeatureVectorVariable > + > +//templates have no
> state. They do not have associated domains. You pass a template to a
> domain, and it returns a feature vector that points to this domain. >
> +trait FeatureTemplateDomain { > + def domain: VectorDomain I see. Need
> domain here to pass into FeatureTemplateVectorVariable constructor. How
> about removing both these methods and having a "def
> newBlankFeatureVectorVar" instead? Do you need the "domain" on its own
> elsewhere? — Reply to this email directly or view it on GitHub.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/factorie/factorie/pull/95/files#r7766247>
> .
>



-- 
 - Alexandre