From bd04eb76eecfecef9a67a1c6a0d467011bce61cf Mon Sep 17 00:00:00 2001 From: Rakesh Kashyap Hanasoge Padmanabha Date: Tue, 4 Apr 2023 17:32:50 -0700 Subject: [PATCH] fix derived feature --- .../main/java/com/linkedin/feathr/common/FeatureTypes.java | 3 ++- .../linkedin/feathr/common/FeatureVariableResolver.java | 6 ++++-- .../TestMemberUrnFeatureDataExtractor.scala | 0 .../derived/functions/SimpleMvelDerivationFunction.scala | 2 +- .../offline/mvel/FeatureVariableResolverFactory.scala | 6 ++++-- .../feathr/offline/AnchoredFeaturesIntegTest.scala | 7 ++++++- gradle.properties | 2 +- 7 files changed, 18 insertions(+), 8 deletions(-) create mode 100644 feathr-impl/src/main/scala/com/linkedin/feathr/offline/anchored/anchorExtractor/TestMemberUrnFeatureDataExtractor.scala diff --git a/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureTypes.java b/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureTypes.java index d30960262..35743bc34 100644 --- a/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureTypes.java +++ b/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureTypes.java @@ -11,5 +11,6 @@ public enum FeatureTypes { TERM_VECTOR, CATEGORICAL_SET, UNSPECIFIED, - TENSOR + TENSOR, + VECTOR } diff --git a/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureVariableResolver.java b/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureVariableResolver.java index bb84152ca..76edf72dc 100644 --- a/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureVariableResolver.java +++ b/feathr-impl/src/main/java/com/linkedin/feathr/common/FeatureVariableResolver.java @@ -18,10 +18,12 @@ public class FeatureVariableResolver extends SimpleValueResolver { private FeatureValue _featureValue; private Optional _mvelContext = Optional.empty(); - public FeatureVariableResolver(FeatureValue featureValue, Optional mvelContext) { + private Optional _shouldConvert; + public FeatureVariableResolver(FeatureValue featureValue, Optional mvelContext, Optional shouldConvert) { super(featureValue); _featureValue = featureValue; _mvelContext = mvelContext; + _shouldConvert = shouldConvert; } @Override @@ -47,7 +49,7 @@ public Object getValue() { throw new IllegalArgumentException("Unexpected feature type: " + _featureValue.getFeatureType().getBasicType()); } // If there is any registered FeatureValue handler that can handle this feature value, return the converted value per request. - if (_mvelContext.isPresent() && _mvelContext.get().canConvertFromAny(fv)) { + if (_mvelContext.isPresent() && _mvelContext.get().canConvertFromAny(fv) && _shouldConvert.isPresent() && _shouldConvert.get()) { return _mvelContext.get().convertFromAny(fv).head(); } else { return fv; diff --git a/feathr-impl/src/main/scala/com/linkedin/feathr/offline/anchored/anchorExtractor/TestMemberUrnFeatureDataExtractor.scala b/feathr-impl/src/main/scala/com/linkedin/feathr/offline/anchored/anchorExtractor/TestMemberUrnFeatureDataExtractor.scala new file mode 100644 index 000000000..e69de29bb diff --git a/feathr-impl/src/main/scala/com/linkedin/feathr/offline/derived/functions/SimpleMvelDerivationFunction.scala b/feathr-impl/src/main/scala/com/linkedin/feathr/offline/derived/functions/SimpleMvelDerivationFunction.scala index 8d9695e2b..e07507b1d 100644 --- a/feathr-impl/src/main/scala/com/linkedin/feathr/offline/derived/functions/SimpleMvelDerivationFunction.scala +++ b/feathr-impl/src/main/scala/com/linkedin/feathr/offline/derived/functions/SimpleMvelDerivationFunction.scala @@ -44,7 +44,7 @@ private[offline] class SimpleMvelDerivationFunction(expression: String, featureN MvelContext.ensureInitialized() // In order to prevent MVEL from barfing if a feature is null, we use a custom variable resolver that understands `Option` - val variableResolverFactory = new FeatureVariableResolverFactory(args, mvelContext) + val variableResolverFactory = new FeatureVariableResolverFactory(args, mvelContext, Some(true)) if (TestFwkUtils.IS_DEBUGGER_ENABLED) { while(TestFwkUtils.DERIVED_FEATURE_COUNTER > 0) { diff --git a/feathr-impl/src/main/scala/com/linkedin/feathr/offline/mvel/FeatureVariableResolverFactory.scala b/feathr-impl/src/main/scala/com/linkedin/feathr/offline/mvel/FeatureVariableResolverFactory.scala index 8f05ac3f0..bc53e54b5 100644 --- a/feathr-impl/src/main/scala/com/linkedin/feathr/offline/mvel/FeatureVariableResolverFactory.scala +++ b/feathr-impl/src/main/scala/com/linkedin/feathr/offline/mvel/FeatureVariableResolverFactory.scala @@ -4,13 +4,15 @@ import com.linkedin.feathr.common.{FeatureValue, FeatureVariableResolver} import com.linkedin.feathr.offline.mvel.plugins.FeathrExpressionExecutionContext import org.mvel2.integration.VariableResolver import org.mvel2.integration.impl.BaseVariableResolverFactory +import collection.JavaConverters._ import java.util.Optional import scala.collection.JavaConverters._ -private[offline] class FeatureVariableResolverFactory(features: Map[String, Option[FeatureValue]], mvelContext: Option[FeathrExpressionExecutionContext]) extends BaseVariableResolverFactory { +private[offline] class FeatureVariableResolverFactory(features: Map[String, Option[FeatureValue]], mvelContext: Option[FeathrExpressionExecutionContext], + shouldConvert: Option[java.lang.Boolean] = Some(false)) extends BaseVariableResolverFactory { - variableResolvers = features.mapValues(x => new FeatureVariableResolver(x.orNull, Optional.ofNullable(mvelContext.orNull))).asInstanceOf[Map[String, VariableResolver]].asJava + variableResolvers = features.mapValues(x => new FeatureVariableResolver(x.orNull, Optional.ofNullable(mvelContext.orNull), Optional.ofNullable(shouldConvert.orNull))).asInstanceOf[Map[String, VariableResolver]].asJava override def isTarget(name: String): Boolean = features.contains(name) diff --git a/feathr-impl/src/test/scala/com/linkedin/feathr/offline/AnchoredFeaturesIntegTest.scala b/feathr-impl/src/test/scala/com/linkedin/feathr/offline/AnchoredFeaturesIntegTest.scala index 15509171b..0b99db004 100644 --- a/feathr-impl/src/test/scala/com/linkedin/feathr/offline/AnchoredFeaturesIntegTest.scala +++ b/feathr-impl/src/test/scala/com/linkedin/feathr/offline/AnchoredFeaturesIntegTest.scala @@ -121,6 +121,8 @@ class AnchoredFeaturesIntegTest extends FeathrIntegTest { | } | } | + | + | | anchor3: { | source: ptSource | key: "IdInObservation" @@ -135,11 +137,14 @@ class AnchoredFeaturesIntegTest extends FeathrIntegTest { |derivations: { | multiply_a_b: "toNumeric(aa) * toNumeric(bb)" | + | xyz:fIntAsCategorical + | | categorical_b: { | key: [foo] | inputs: { foo_b: { key: foo, feature: bb } } | definition: "toCategorical(foo_b)" | } + | |} """.stripMargin.format(trainingData, featureData) @BeforeClass @@ -166,7 +171,7 @@ class AnchoredFeaturesIntegTest extends FeathrIntegTest { */ @Test def testSingleKeyJoinWithDifferentFeatureTypes(): Unit = { - val selectedColumns = Seq("x", "aa", "bb", "cc", "dd", "ee", "ee2", "ff", "multiply_a_b", "categorical_b") // , "z") + val selectedColumns = Seq("x", "aa", "bb", "cc", "dd", "ee", "ee2", "ff", "multiply_a_b", "categorical_b", "xyz") val featureJoinConf = s""" | diff --git a/gradle.properties b/gradle.properties index 0433134fb..061616df2 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ -version=1.0.2-rc3 +version=1.0.2-rc4 SONATYPE_AUTOMATIC_RELEASE=true POM_ARTIFACT_ID=feathr_2.12