Merge pull request #409 from PavlidisLab/fix-go-term-recommendations

Fix GO term recommendations

docs/customization.md

@@ -116,6 +116,29 @@ where taxon_id = 10090;
 
 Every time new model systems are added to the application, they will have to be activated in this manner.
 
+## GO term recommendation
+
+Users can receive recommended terms based on the TIER1 and TIER2 genes they have added to their profiles.
+
+The recommendation algorithm works as follows:
+
+1. Retrieve GO terms associated to all TIER1 and TIER2 genes
+2. Retrieve all the descendants of these terms
+3. For each term, compute how many TIER1 or TIER2 genes they are associated either directly or indirectly via their
+   descendants
+4. Keep terms that are not already on the user profile and that mention at least 2 TIER1 or TIER2 genes
+5. Exclude terms with more than 50 associated genes
+6. Retain terms that have at least one novel gene that is not on the user's profile
+7. Retain most specific terms if a given term and its descendant is recommended
+
+You can adjust the number of overlapping TIER1 or TIER2 genes and the maximum size of a GO term by setting the
+following:
+
+```ini
+rdp.settings.go-term-min-overlap=2 # new in 1.5.8
+rdp.settings.go-term-size-limit=50
+```
+
 ### Customizing taxon appearance (new in 1.5.5)
 
 By default, taxon are rendered using the common name in title case. The only exception is for *Homo sapiens* which

src/main/java/ubc/pavlab/rdp/controllers/UserController.java

@@ -3,14 +3,17 @@
 import lombok.Builder;
 import lombok.Data;
 import lombok.EqualsAndHashCode;
+import lombok.Value;
 import lombok.extern.apachecommons.CommonsLog;
 import org.apache.commons.lang3.StringUtils;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.MessageSource;
+import org.springframework.context.MessageSourceResolvable;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.InvalidMediaTypeException;
 import org.springframework.http.MediaType;
 import org.springframework.http.ResponseEntity;
+import org.springframework.lang.Nullable;
 import org.springframework.security.access.annotation.Secured;
 import org.springframework.security.authentication.BadCredentialsException;
 import org.springframework.stereotype.Controller;
@@ -498,23 +501,45 @@ public Object getTermsForTaxon( @PathVariable Integer taxonId,
         return goIds.stream().collect( toNullableMap( identity(), goId -> goService.getTerm( goId ) == null ? null : userService.convertTerm( user, taxon, goService.getTerm( goId ) ) ) );
     }
 
+    @Value
+    static class RecommendedTermsModel {
+        /**
+         * List of recommended GO terms.
+         */
+        public Collection<UserTerm> recommendedTerms;
+        /**
+         * Feedback to be displayed or null if no feedback is available.
+         */
+        @Nullable
+        public String feedback;
+    }
+
     @ResponseBody
     @GetMapping(value = "/user/taxon/{taxonId}/term/recommend", produces = MediaType.APPLICATION_JSON_VALUE)
     public Object getRecommendedTermsForTaxon( @PathVariable Integer taxonId,
-                                               @RequestParam(required = false) List<Integer> geneIds ) {
+                                               @RequestParam(required = false) List<Integer> geneIds,
+                                               Locale locale ) {
         Taxon taxon = taxonService.findById( taxonId );
         if ( taxon == null ) {
             return ResponseEntity.notFound().build();
         }
 
-        Set<GeneInfo> genes;
+        User user = userService.findCurrentUser();
+
+        Collection<UserTerm> recommendedTerms;
+        List<MessageSourceResolvable> feedback = new ArrayList<>();
         if ( geneIds != null ) {
-            genes = new HashSet<>( geneService.load( geneIds ) );
+            Set<? extends Gene> genes = new HashSet<>( geneService.load( geneIds ) );
+            recommendedTerms = userService.recommendTerms( user, genes, taxon, feedback );
         } else {
-            genes = Collections.emptySet();
+            recommendedTerms = userService.recommendTerms( user, taxon, feedback );
         }
 
-        return userService.recommendTerms( userService.findCurrentUser(), genes, taxon );
+        String formattedFeedback = feedback.isEmpty() ? null : feedback.stream()
+                .map( f -> messageSource.getMessage( f, locale ) )
+                .collect( Collectors.joining( "\n" ) );
+
+        return new RecommendedTermsModel( recommendedTerms, formattedFeedback );
     }
 
     private Set<TierType> getManualTiers() {

src/main/java/ubc/pavlab/rdp/repositories/GeneOntologyTermInfoRepository.java

@@ -6,6 +6,7 @@
 import org.springframework.util.LinkedMultiValueMap;
 import org.springframework.util.MultiValueMap;
 import ubc.pavlab.rdp.model.GeneOntologyTermInfo;
+import ubc.pavlab.rdp.model.Taxon;
 
 import java.util.*;
 import java.util.concurrent.locks.Lock;
@@ -198,6 +199,40 @@ public long count() {
         }
     }
 
+    /**
+     * Count the number of terms for the given taxon;
+     */
+    public long countByTaxon( Taxon taxon ) {
+        Lock lock = rwLock.readLock();
+        try {
+            lock.lock();
+            return termsByIdOrAlias.values().stream()
+                    .distinct()
+                    .map( GeneOntologyTermInfo::getDirectGeneIdsByTaxonId )
+                    .filter( m -> m.containsKey( taxon.getId() ) )
+                    .count();
+        } finally {
+            lock.unlock();
+        }
+    }
+
+    /**
+     * Count the number of term-gene associations for the given taxon.
+     */
+    public long countGeneAssociationsByTaxon( Taxon taxon ) {
+        Lock lock = rwLock.readLock();
+        try {
+            lock.lock();
+            return termsByIdOrAlias.values().stream()
+                    .distinct()
+                    .map( GeneOntologyTermInfo::getDirectGeneIdsByTaxonId )
+                    .mapToLong( m -> m.getOrDefault( taxon.getId(), Collections.emptyList() ).size() )
+                    .sum();
+        } finally {
+            lock.unlock();
+        }
+    }
+
     @Override
     public void deleteById( String id ) {
         // FIXME: we should acquire a read lock here and promote it to a write lock if the element exists, but I don't

src/main/java/ubc/pavlab/rdp/services/GOService.java

@@ -24,6 +24,10 @@ public interface GOService {
 
     long count();
 
+    long countByTaxon( Taxon taxon );
+
+    long countGeneAssociationsByTaxon( Taxon taxon );
+
     Collection<GeneOntologyTermInfo> getDescendants( GeneOntologyTermInfo entry );
 
     Collection<GeneOntologyTermInfo> getAncestors( GeneOntologyTermInfo entry );

src/main/java/ubc/pavlab/rdp/services/GOServiceImpl.java

@@ -279,12 +279,12 @@ public List<SearchResult<GeneOntologyTermInfo>> search( String queryString, Taxo
 
     @Override
     public long getSizeInTaxon( GeneOntologyTermInfo t, Taxon taxon ) {
-        Collection<GeneOntologyTermInfo> descendants = getDescendants( t );
+        Collection<GeneOntologyTermInfo> descendants = new HashSet<>( getDescendants( t ) );
         descendants.add( t );
         return descendants.stream()
+                .flatMap( term -> term.getDirectGeneIdsByTaxonId().getOrDefault( taxon.getId(), Collections.emptyList() ).stream() )
                 .distinct()
-                .mapToLong( term -> term.getDirectGeneIdsByTaxonId().getOrDefault( taxon.getId(), Collections.emptyList() ).size() )
-                .sum();
+                .count();
     }
 
     @Override
@@ -360,13 +360,23 @@ public long count() {
         return goRepository.count();
     }
 
+    @Override
+    public long countByTaxon( Taxon taxon ) {
+        return goRepository.countByTaxon( taxon );
+    }
+
+    @Override
+    public long countGeneAssociationsByTaxon( Taxon taxon ) {
+        return goRepository.countGeneAssociationsByTaxon( taxon );
+    }
+
     @Override
     public Collection<GeneOntologyTermInfo> getDescendants( GeneOntologyTermInfo entry ) {
         StopWatch timer = StopWatch.createStarted();
         Lock lock = rwLock.readLock();
         try {
             lock.lock();
-            return getDescendantsInternal( entry );
+            return Collections.unmodifiableCollection( getDescendantsInternal( entry ) );
         } finally {
             lock.unlock();
             if ( timer.getTime( TimeUnit.MILLISECONDS ) > 1000 ) {
@@ -386,6 +396,9 @@ private Set<GeneOntologyTermInfo> getDescendantsInternal( GeneOntologyTermInfo e
             results.add( child );
             results.addAll( getDescendantsInternal( child ) );
         }
+        if ( results.remove( entry ) ) {
+            log.warn( String.format( "%s is its own descendant, removing it to prevent cycles.", entry ) );
+        }
         descendantsCache.put( entry, results );
         return results;
     }
@@ -479,7 +492,7 @@ public Collection<GeneOntologyTermInfo> getAncestors( GeneOntologyTermInfo term
         Lock lock = rwLock.readLock();
         try {
             lock.lock();
-            return getAncestorsInternal( term );
+            return Collections.unmodifiableCollection( getAncestorsInternal( term ) );
         } finally {
             lock.unlock();
         }
@@ -495,6 +508,9 @@ private Collection<GeneOntologyTermInfo> getAncestorsInternal( GeneOntologyTermI
             results.add( parent );
             results.addAll( getAncestorsInternal( parent ) );
         }
+        if ( results.remove( term ) ) {
+            log.warn( String.format( "%s is its own ancestor, removing it to prevent cycle.", term ) );
+        }
         ancestorsCache.put( term, results );
         return results;
     }

src/main/java/ubc/pavlab/rdp/services/UserService.java

@@ -1,7 +1,9 @@
 package ubc.pavlab.rdp.services;
 
+import org.springframework.context.MessageSourceResolvable;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;
+import org.springframework.lang.Nullable;
 import org.springframework.security.authentication.BadCredentialsException;
 import ubc.pavlab.rdp.exception.TokenException;
 import ubc.pavlab.rdp.model.*;
@@ -161,12 +163,12 @@ public interface UserService {
      * <p>
      * The recommendation are based on the user's {@link TierType#MANUAL}  gene set.
      */
-    Collection<UserTerm> recommendTerms( User user, Taxon taxon );
+    Collection<UserTerm> recommendTerms( User user, Taxon taxon, @Nullable List<MessageSourceResolvable> feedback );
 
     /**
      * Recommend terms for a user using a supplied gene set which might differ from the user's.
      */
-    Collection<UserTerm> recommendTerms( User user, Set<? extends Gene> genes, Taxon taxon );
+    Collection<UserTerm> recommendTerms( User user, Set<? extends Gene> genes, Taxon taxon, @Nullable List<MessageSourceResolvable> feedback );
 
     User updateTermsAndGenesInTaxon( User user,
                                      Taxon taxon,