Skip to content

Commit

Permalink
Merge pull request #4415 from inception-project/bugfix/4414-Number-of…
Browse files Browse the repository at this point in the history 
…-new-suggestions-not-displayed-correctly

#4414 - Number of new suggestions not displayed correctly
  • Loading branch information
@reckart
reckart authored Dec 28, 2023
2 parents d4b0399 + ed48cd6 commit 66c61b3
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 36 deletions.
79 changes: 58 additions & 21 deletions ...-api/src/main/java/de/tudarmstadt/ukp/inception/recommendation/api/model/Predictions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@
import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableSet;
import static java.util.Comparator.comparingInt;
import static java.util.stream.Collectors.toList;

import java.io.Serializable;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand All @@ -35,6 +35,8 @@

import org.apache.commons.lang3.Validate;
import org.apache.uima.cas.text.AnnotationPredicates;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationDocument;
import de.tudarmstadt.ukp.clarin.webanno.model.AnnotationLayer;
Expand All @@ -52,6 +54,8 @@
public class Predictions
implements Serializable
{
private static final Logger LOG = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

private static final long serialVersionUID = -1598768729246662885L;

private final int generation;
Expand All @@ -69,7 +73,9 @@ public class Predictions
// session, the pool of IDs of positive integer values is never exhausted.
private int nextId;

private int newSuggestionCount = 0;
private int addedSuggestionCount = 0;
private int agedSuggestionCount = 0;
private int removedSuggestionCount = 0;

public Predictions(User aSessionOwner, String aDataOwner, Project aProject)
{
Expand Down Expand Up @@ -183,7 +189,7 @@ private <T extends AnnotationSuggestion> List<T> getFlattenedPredictions(Class<T
f.getValue().getWindowEnd(), windowBegin, windowEnd))
.sorted(comparingInt(e2 -> e2.getValue().getWindowBegin())) //
.map(Map.Entry::getValue) //
.collect(toList());
.toList();
}
}

Expand All @@ -206,32 +212,56 @@ public Optional<AnnotationSuggestion> getPredictionByVID(SourceDocument aDocumen
}
}

/**
* @param aPredictions
* list of sentences containing recommendations
*/
public void putPredictions(List<AnnotationSuggestion> aPredictions)
public void putSuggestions(int aAdded, int aRemoved, int aAged,
List<AnnotationSuggestion> aSuggestions)
{
synchronized (predictionsLock) {
for (var prediction : aPredictions) {
addedSuggestionCount += aAdded;
agedSuggestionCount += aAged;
removedSuggestionCount += aRemoved;

var ageZeroSuggestions = 0;
for (var suggestion : aSuggestions) {
// Assign ID to predictions that do not have an ID yet
if (prediction.getId() == AnnotationSuggestion.NEW_ID) {
prediction = prediction.assignId(nextId);
if (suggestion.getId() == AnnotationSuggestion.NEW_ID) {
suggestion = suggestion.assignId(nextId);
nextId++;
if (nextId < 0) {
throw new IllegalStateException(
"Annotation suggestion ID overflow. Restart session.");
}
}

var xid = new ExtendedId(suggestion);
var byDocument = idxDocuments.computeIfAbsent(suggestion.getDocumentName(),
$ -> new HashMap<>());
byDocument.put(xid, suggestion);

if (suggestion.getAge() == 0) {
ageZeroSuggestions++;
}
}

if (aAdded != ageZeroSuggestions) {
LOG.warn("Expected [{}] age-zero suggestions but found [{}]", aAdded,
ageZeroSuggestions);
}
}
}

public void inheritSuggestions(List<AnnotationSuggestion> aPredictions)
{
synchronized (predictionsLock) {
for (var prediction : aPredictions) {
if (prediction.getId() == AnnotationSuggestion.NEW_ID) {
throw new IllegalStateException(
"Inherited suggestions must already have an ID");
}

var xid = new ExtendedId(prediction);
var byDocument = idxDocuments.computeIfAbsent(prediction.getDocumentName(),
$ -> new HashMap<>());
byDocument.put(xid, prediction);

if (prediction.getAge() == 0) {
newSuggestionCount++;
}
}
}
}
Expand All @@ -250,12 +280,12 @@ public boolean isEmpty()

public boolean hasNewSuggestions()
{
return newSuggestionCount > 0;
return addedSuggestionCount > 0;
}

public int getNewSuggestionCount()
{
return newSuggestionCount;
return addedSuggestionCount;
}

public int size()
Expand Down Expand Up @@ -286,7 +316,7 @@ public List<SpanSuggestion> getAlternativeSuggestions(SpanSuggestion aSuggestion
.filter(f -> f.getValue().getEnd() == aSuggestion.getEnd()) //
.filter(f -> f.getValue().getFeature().equals(aSuggestion.getFeature())) //
.map(Map.Entry::getValue) //
.collect(toList());
.toList();
}
}

Expand Down Expand Up @@ -321,7 +351,7 @@ public List<SpanSuggestion> getPredictionsByTokenAndFeature(String aDocumentName
.filter(f -> f.getValue().getEnd() == aEnd) //
.filter(f -> f.getValue().getFeature().equals(aFeature)) //
.map(Map.Entry::getValue) //
.collect(toList());
.toList();
}
}

Expand All @@ -333,7 +363,7 @@ public List<AnnotationSuggestion> getPredictionsByRecommenderAndDocument(
return byDocument.entrySet().stream() //
.filter(f -> f.getKey().getRecommenderId() == (long) aRecommender.getId())
.map(Map.Entry::getValue) //
.collect(toList());
.toList();
}
}

Expand All @@ -343,7 +373,7 @@ public List<AnnotationSuggestion> getPredictionsByDocument(String aDocumentName)
var byDocument = idxDocuments.getOrDefault(aDocumentName, emptyMap());
return byDocument.entrySet().stream() //
.map(Map.Entry::getValue) //
.collect(toList());
.toList();
}
}

Expand All @@ -354,6 +384,13 @@ public void markDocumentAsPredictionCompleted(SourceDocument aDocument)
}
}

public int getDocumentsSeenCount()
{
synchronized (seenDocumentsForPrediction) {
return seenDocumentsForPrediction.size();
}
}

Set<String> documentsSeen()
{
return unmodifiableSet(seenDocumentsForPrediction);
Expand Down
10 changes: 5 additions & 5 deletions .../src/test/java/de/tudarmstadt/ukp/inception/recommendation/api/model/PredictionsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ void testGetGroupedPredictions() throws Exception
var winBegin = sentences.get(Math.round(sentences.size() * 0.25f)).getBegin();
var winEnd = sentences.get(Math.round(sentences.size() * 0.75f)).getEnd();

sut.putPredictions(generatePredictions(100, 1, 100));
sut.inheritSuggestions(generatePredictions(100, 1, 100));
assertThat(sut.size()).isEqualTo(10000);

var groups = sut.getGroupedPredictions(SpanSuggestion.class, "doc10", layer, winBegin,
Expand All @@ -93,7 +93,7 @@ void timeGetGroupedPredictions() throws Exception

sut = new Predictions(user, user.getUsername(), project);
var generatedPredictions = generatePredictions(10_000, 1, 1_000);
sut.putPredictions(generatedPredictions);
sut.inheritSuggestions(generatedPredictions);
var documents = generatedPredictions.stream() //
.map(AnnotationSuggestion::getDocumentName) //
.distinct() //
Expand All @@ -114,7 +114,7 @@ void thatIdsAreAssigned() throws Exception
{
var doc = "doc";
sut = new Predictions(user, user.getUsername(), project);
sut.putPredictions(asList( //
sut.putSuggestions(1, 0, 0, asList( //
SpanSuggestion.builder() //
.withId(AnnotationSuggestion.NEW_ID) //
.withDocumentName(doc) //
Expand All @@ -126,12 +126,12 @@ void thatIdsAreAssigned() throws Exception

var inheritedPredictions = sut.getPredictionsByDocument(doc);
sut = new Predictions(sut);
sut.putPredictions(asList( //
sut.putSuggestions(1, 0, 0, asList( //
SpanSuggestion.builder() //
.withId(AnnotationSuggestion.NEW_ID) //
.withDocumentName(doc) //
.build()));
sut.putPredictions(inheritedPredictions);
sut.inheritSuggestions(inheritedPredictions);

assertThat(sut.getPredictionsByDocument(doc)) //
.extracting(AnnotationSuggestion::getId) //
Expand Down
28 changes: 20 additions & 8 deletions ...n/java/de/tudarmstadt/ukp/inception/recommendation/service/RecommendationServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1829,7 +1829,7 @@ private void inheritSuggestionsAtRecommenderLevel(Predictions predictions, CAS a
predictions.log(LogMessage.info(aRecommender.getName(),
"Inherited [%d] predictions from previous run", suggestions.size()));

predictions.putPredictions(suggestions);
predictions.inheritSuggestions(suggestions);
}

/**
Expand All @@ -1848,7 +1848,7 @@ private void inheritSuggestionsAtDocumentLevel(Project aProject, SourceDocument
LOG.debug("[{}]({}) for user [{}] on document {} in project {} inherited {} predictions",
"ALL", "--", aUser.getUsername(), aDocument, aProject, suggestions.size());

aNewPredictions.putPredictions(suggestions);
aNewPredictions.inheritSuggestions(suggestions);
aNewPredictions.markDocumentAsPredictionCompleted(aDocument);
}

Expand Down Expand Up @@ -1888,6 +1888,9 @@ void generateSuggestions(Predictions aIncomingPredictions, RecommenderContext aC
generatedSuggestions.size(), predictedRange, reconciliationResult.added,
reconciliationResult.removed, reconciliationResult.aged));
var suggestions = reconciliationResult.suggestions;
var added = reconciliationResult.added;
var removed = reconciliationResult.removed;
var aged = reconciliationResult.aged;

// Inherit suggestions that are outside the range which was predicted. Note that the engine
// might actually predict a different range from what was requested. If the prediction
Expand All @@ -1906,6 +1909,10 @@ void generateSuggestions(Predictions aIncomingPredictions, RecommenderContext aC
aIncomingPredictions.log(LogMessage.info(recommender.getName(),
"Inherited [%d] predictions from previous run", inheritableSuggestions.size()));

for (var suggestion : inheritableSuggestions) {
aged++;
suggestion.incrementAge();
}
suggestions.addAll(inheritableSuggestions);
}

Expand All @@ -1917,7 +1924,7 @@ void generateSuggestions(Predictions aIncomingPredictions, RecommenderContext aC
groupedSuggestions, 0, aOriginalCas.getDocumentText().length());
// FIXME calculateRelationSuggestionVisibility?

aIncomingPredictions.putPredictions(suggestions);
aIncomingPredictions.putSuggestions(added, removed, aged, suggestions);
}

static ReconciliationResult reconcile(Predictions aActivePredictions, SourceDocument aDocument,
Expand All @@ -1943,11 +1950,9 @@ static ReconciliationResult reconcile(Predictions aActivePredictions, SourceDocu
for (var newSuggestion : aNewProtoSuggestions) {
var existingSuggestions = existingSuggestionsByPosition
.getOrDefault(newSuggestion.getPosition(), emptyList()).stream() //
.filter(s -> Objects.equals(s.getLabel(), newSuggestion.getLabel()) && //
s.getScore() == newSuggestion.getScore() && //
Objects.equals(s.getScoreExplanation(),
newSuggestion.getScoreExplanation()))
.collect(toList());
.filter(s -> matchesForReconciliation(newSuggestion, s)) //
.limit(2) // One to use, the second to warn that there was more than one
.toList();

if (existingSuggestions.isEmpty()) {
addedSuggestions.add(newSuggestion);
Expand Down Expand Up @@ -1978,6 +1983,13 @@ static ReconciliationResult reconcile(Predictions aActivePredictions, SourceDocu
agedSuggestionsCount, finalSuggestions);
}

private static boolean matchesForReconciliation(AnnotationSuggestion aNew,
AnnotationSuggestion aExisting)
{
return aNew.getRecommenderId() == aExisting.getRecommenderId() && //
Objects.equals(aExisting.getLabel(), aNew.getLabel());
}

static List<AnnotationSuggestion> extractSuggestions(int aGeneration, CAS aOriginalCas,
CAS aPredictionCas, SourceDocument aDocument, Recommender aRecommender)
{
Expand Down
3 changes: 2 additions & 1 deletion ...ation/src/main/java/de/tudarmstadt/ukp/inception/recommendation/tasks/PredictionTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ public void execute()
appEventPublisher.publishEvent(RecommenderTaskNotificationEvent
.builder(this, project, sessionOwner.getUsername()) //
.withMessage(LogMessage.info(this,
predictions.getNewSuggestionCount() + " new predictions available")) //
predictions.getNewSuggestionCount() + " new predictions available" //
+ " (some may be hidden/merged)")) //
.build());
}
else {
Expand Down
2 changes: 1 addition & 1 deletion ...va/de/tudarmstadt/ukp/inception/recommendation/service/RecommendationServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ void testReconciliation() throws Exception
.withRecommender(rec) //
.build());
var activePredictions = new Predictions(sessionOwner, sessionOwner.getUsername(), project);
activePredictions.putPredictions(existingSuggestions);
activePredictions.inheritSuggestions(existingSuggestions);

var newSuggestions = Arrays.<AnnotationSuggestion> asList( //
SpanSuggestion.builder() //
Expand Down

0 comments on commit 66c61b3

Please sign in to comment.