Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[DROOLS-6627] Verify Scorecard reasoncode when result is null #3875

Merged
merged 23 commits into from
Oct 6, 2021
+41 −13
Merged

[DROOLS-6627] Verify Scorecard reasoncode when result is null #3875

Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3fa1ab8
[DROOLS-6625] Managing missing "required" input data
gitgabrio Sep 29, 2021
5753632
[DROOLS-6625] Managing best-effort conversion of input data
gitgabrio Sep 29, 2021
680cdd3
[DROOLS-6625] Managing invalid values - TODO: integration tests
gitgabrio Sep 29, 2021
25f4fa2
[DROOLS-6625] Managing invalid values
gitgabrio Sep 29, 2021
0e7d37e
[DROOLS-6625] Managing missing values
gitgabrio Sep 30, 2021
556fa70
Merge remote-tracking branch 'origin/7.x' into DROOLS-6625
gitgabrio Sep 30, 2021
f01da9d
[DROOLS-6625] Validate input data
gitgabrio Sep 30, 2021
0888aa8
[DROOLS-6635] Move testing sources in specific files
gitgabrio Oct 1, 2021
4bad52d
Merge remote-tracking branch 'origin/7.x' into DROOLS-6625
gitgabrio Oct 1, 2021
ac363eb
[DROOLS-6625] Fix merge with base branch
gitgabrio Oct 1, 2021
f7b6a53
[DROOLS-6625] Fix as per PR suggestion
gitgabrio Oct 1, 2021
f9ead51
Merge remote-tracking branch 'origin/7.x' into DROOLS-6625
gitgabrio Oct 4, 2021
60079cc
Merge remote-tracking branch 'gitgabrio/DROOLS-6625' into DROOLS-6635
gitgabrio Oct 4, 2021
a2ec9c9
[DROOLS-6635] Fix merge
gitgabrio Oct 4, 2021
d565f47
[DROOLS-6627] Verify Scorecard reasoncode when result is null
gitgabrio Oct 4, 2021
3ef7c98
Merge remote-tracking branch 'origin/7.x' into DROOLS-6635
gitgabrio Oct 5, 2021
d0e06a0
[DROOLS-6635] Fix merge with 7.x
gitgabrio Oct 5, 2021
70268eb
Merge remote-tracking branch 'origin/7.x' into DROOLS-6627
gitgabrio Oct 5, 2021
b0441d7
Merge remote-tracking branch 'gitgabrio/DROOLS-6635' into DROOLS-6627
gitgabrio Oct 5, 2021
01b40ce
[DROOLS-6627] Fix merge
gitgabrio Oct 5, 2021
3e004f9
Merge remote-tracking branch 'origin/7.x' into DROOLS-6627
gitgabrio Oct 5, 2021
3cf85fa
Merge remote-tracking branch 'origin/7.x' into DROOLS-6627
gitgabrio Oct 6, 2021
ae9afd8
[DROOLS-6627] Fixed as per PR request
gitgabrio Oct 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...ols-common/src/main/java/org/kie/pmml/models/drools/commons/model/KiePMMLDroolsModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public abstract class KiePMMLDroolsModel extends KiePMMLModel implements IsDrool
private static final Logger logger = LoggerFactory.getLogger(KiePMMLDroolsModel.class);

private static final AgendaEventListener agendaEventListener = getAgendaEventListener(logger);
private static final long serialVersionUID = 5471400949048174357L;

/**
* Map between the original field name and the generated type.
Expand All @@ -75,10 +76,12 @@ public Object evaluate(final Object knowledgeBase, Map<String, Object> requestDa
String fullClassName = this.getClass().getName();
String packageName = fullClassName.contains(".") ?
fullClassName.substring(0, fullClassName.lastIndexOf('.')) : "";
Map<String, Object> holder = new HashMap<>();
KiePMMLSessionUtils.Builder builder = KiePMMLSessionUtils.builder((KieBase) knowledgeBase, name, packageName,
toReturn)
.withObjectsInSession(requestData, fieldTypeMap)
.withOutputFieldsMap(outputFieldsMap);
.withOutputFieldsMap(holder);
outputFieldsMap = holder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? I don't usually like a method that changes the instance of a field. Isn't possible to do
outpitFieldsMap.putAll(output)
?
Or do a clear() if necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielezonca
done

if (logger.isDebugEnabled()) {
builder = builder.withAgendaEventListener(agendaEventListener);
}
Expand Down
20 changes: 16 additions & 4 deletions .../test/java/org/kie/pmml/models/drools/scorecard/tests/SimpleScorecardCategoricalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.kie.pmml.models.drools.scorecard.tests;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
Expand All @@ -35,6 +34,8 @@
import org.kie.pmml.api.runtime.PMMLRuntime;
import org.kie.pmml.models.tests.AbstractPMMLTest;

import static org.junit.Assert.assertFalse;

@RunWith(Parameterized.class)
public class SimpleScorecardCategoricalTest extends AbstractPMMLTest {

Expand All @@ -43,7 +44,7 @@ public class SimpleScorecardCategoricalTest extends AbstractPMMLTest {
private static final String TARGET_FIELD = "Score";
private static final String REASON_CODE1_FIELD = "Reason Code 1";
private static final String REASON_CODE2_FIELD = "Reason Code 2";
private static final String[] CATEGORY = new String[] { "classA", "classB", "classC", "classD", "classE", "NA" };
private static final String[] CATEGORY = new String[]{"classA", "classB", "classC", "classD", "classE", "NA"};
private static PMMLRuntime pmmlRuntime;

private String input1;
Expand All @@ -52,15 +53,16 @@ public class SimpleScorecardCategoricalTest extends AbstractPMMLTest {
private String reasonCode1;
private String reasonCode2;

public SimpleScorecardCategoricalTest(String input1, String input2, double score, String reasonCode1, String reasonCode2) {
public SimpleScorecardCategoricalTest(String input1, String input2, double score, String reasonCode1,
String reasonCode2) {
this.input1 = input1;
this.input2 = input2;
this.score = score;
this.reasonCode1 = reasonCode1;
this.reasonCode2 = reasonCode2;
}

@BeforeClass
@BeforeClass
public static void setupClass() {
pmmlRuntime = getPMMLRuntime(FILE_NAME);
}
Expand Down Expand Up @@ -93,6 +95,16 @@ public void testSimpleScorecardCategoricalVerifyNoException() {
getSamples().stream().map(sample -> evaluate(pmmlRuntime, sample, MODEL_NAME)).forEach(Assert::assertNotNull);
}

@Test
public void testSimpleScorecardCategoricalVerifyNoReasonCodeWithoutScore() {
getSamples().stream().map(sample -> evaluate(pmmlRuntime, sample, MODEL_NAME))
.filter(pmml4Result -> pmml4Result.getResultVariables().get(TARGET_FIELD) == null)
.forEach(pmml4Result -> {
assertFalse(pmml4Result.getResultVariables().containsKey(REASON_CODE1_FIELD));
assertFalse(pmml4Result.getResultVariables().containsKey(REASON_CODE2_FIELD));
});
}

private List<Map<String, Object>> getSamples() {
return IntStream.range(0, 10).boxed().map(i -> new HashMap<String, Object>() {{
put("input1", CATEGORY[i % CATEGORY.length]);
Expand Down
16 changes: 10 additions & 6 deletions ...recard-model/src/main/java/org/kie/pmml/models/scorecard/model/KiePMMLScorecardModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -66,12 +67,15 @@ public Object evaluate(final Object knowledgeBase, final Map<String, Object> req
if (localTransformations != null) {
derivedFields.addAll(localTransformations.getDerivedFields());
}
return characteristics.evaluate(defineFunctions, derivedFields, kiePMMLOutputFields, requestData,
outputFieldsMap,
initialScore,
reasonCodeAlgorithm,
useReasonCodes,
baselineScore).orElse(null);
Map<String, Object> holder = new HashMap<>();
Object toReturn = characteristics.evaluate(defineFunctions, derivedFields, kiePMMLOutputFields, requestData,
holder,
initialScore,
reasonCodeAlgorithm,
useReasonCodes,
baselineScore).orElse(null);
outputFieldsMap = holder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielezonca
done

return toReturn;
}

@Override
Expand Down
21 changes: 17 additions & 4 deletions ...sts/src/test/java/org/kie/pmml/models/scorecard/tests/SimpleScorecardCategoricalTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
import org.kie.pmml.api.runtime.PMMLRuntime;
import org.kie.pmml.models.tests.AbstractPMMLTest;

import static org.junit.Assert.assertFalse;

@RunWith(Parameterized.class)
public class SimpleScorecardCategoricalTest extends AbstractPMMLTest {

Expand All @@ -42,7 +44,7 @@ public class SimpleScorecardCategoricalTest extends AbstractPMMLTest {
private static final String TARGET_FIELD = "Score";
private static final String REASON_CODE1_FIELD = "Reason Code 1";
private static final String REASON_CODE2_FIELD = "Reason Code 2";
private static final String[] CATEGORY = new String[] { "classA", "classB", "classC", "classD", "classE", "NA" };
private static final String[] CATEGORY = new String[]{"classA", "classB", "classC", "classD", "classE", "NA"};
private static PMMLRuntime pmmlRuntime;

private String input1;
Expand All @@ -51,7 +53,8 @@ public class SimpleScorecardCategoricalTest extends AbstractPMMLTest {
private String reasonCode1;
private String reasonCode2;

public SimpleScorecardCategoricalTest(String input1, String input2, double score, String reasonCode1, String reasonCode2) {
public SimpleScorecardCategoricalTest(String input1, String input2, double score, String reasonCode1,
String reasonCode2) {
this.input1 = input1;
this.input2 = input2;
this.score = score;
Expand Down Expand Up @@ -89,7 +92,18 @@ public void testSimpleScorecardCategorical() {

@Test
public void testSimpleScorecardCategoricalVerifyNoException() {
getSamples().stream().map(sample -> evaluate(pmmlRuntime, sample, MODEL_NAME)).forEach(Assert::assertNotNull);
getSamples().stream().map(sample -> evaluate(pmmlRuntime, sample, MODEL_NAME))
.forEach(Assert::assertNotNull);
}

@Test
public void testSimpleScorecardCategoricalVerifyNoReasonCodeWithoutScore() {
getSamples().stream().map(sample -> evaluate(pmmlRuntime, sample, MODEL_NAME))
.filter(pmml4Result -> pmml4Result.getResultVariables().get(TARGET_FIELD) == null)
.forEach(pmml4Result -> {
assertFalse(pmml4Result.getResultVariables().containsKey(REASON_CODE1_FIELD));
assertFalse(pmml4Result.getResultVariables().containsKey(REASON_CODE2_FIELD));
});
}

private List<Map<String, Object>> getSamples() {
Expand All @@ -98,5 +112,4 @@ private List<Map<String, Object>> getSamples() {
put("input2", CATEGORY[Math.abs(CATEGORY.length - i) % CATEGORY.length]);
}}).collect(Collectors.toList());
}

}