From e871a64e10c4260a355d34c1c03e1f1b3fb37148 Mon Sep 17 00:00:00 2001 From: Stefan Hahmann Date: Thu, 5 Dec 2024 12:12:08 +0100 Subject: [PATCH 1/4] Let DendrogramView directly extend JFrame instead of holding a JFrame instance * Remove the listeners from the components on close() * Reasoning: before this commit, if the user had closed the JFrame, the instance of it was still kept, since the DendrogramView object was never cleaned up. Thus, also the listeners were still unnecessarily active, which lead to bugs in certain situations --- .../clustering/ClusterLineagesController.java | 2 +- .../mamut/clustering/ui/DendrogramView.java | 97 +++++++++++-------- 2 files changed, 57 insertions(+), 42 deletions(-) diff --git a/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java b/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java index 7d5aa4604..495919eb0 100644 --- a/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java +++ b/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java @@ -286,7 +286,7 @@ private void showDendrogram( final HierarchicalClusteringResult< BranchSpotTree String header = "Dendrogram of hierarchical clustering of lineages
" + getParameters() + ""; DendrogramView< BranchSpotTree > dendrogramView = new DendrogramView<>( hierarchicalClusteringResult, header, referenceModel, prefs, referenceProjectModel.getProjectName() ); - dendrogramView.show(); + dendrogramView.setVisible( true ); } private HierarchicalClusteringResult< BranchSpotTree > classifyLineageTrees( final List< BranchSpotTree > roots, diff --git a/src/main/java/org/mastodon/mamut/clustering/ui/DendrogramView.java b/src/main/java/org/mastodon/mamut/clustering/ui/DendrogramView.java index c74d9657c..103f02f70 100644 --- a/src/main/java/org/mastodon/mamut/clustering/ui/DendrogramView.java +++ b/src/main/java/org/mastodon/mamut/clustering/ui/DendrogramView.java @@ -28,16 +28,19 @@ */ package org.mastodon.mamut.clustering.ui; -import net.miginfocom.swing.MigLayout; -import org.mastodon.mamut.clustering.util.HierarchicalClusteringResult; -import org.mastodon.mamut.model.Model; -import org.mastodon.model.tag.TagSetModel; -import org.mastodon.model.tag.TagSetStructure; -import org.mastodon.ui.util.ExtensionFileFilter; -import org.mastodon.ui.util.FileChooser; -import org.scijava.prefs.PrefService; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import java.awt.Color; +import java.awt.Desktop; +import java.awt.Dimension; +import java.awt.Toolkit; +import java.awt.event.ActionListener; +import java.awt.event.WindowAdapter; +import java.awt.event.WindowEvent; +import java.io.File; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.ArrayList; +import java.util.List; +import java.util.function.ObjIntConsumer; import javax.swing.BorderFactory; import javax.swing.JButton; @@ -50,24 +53,25 @@ import javax.swing.JPopupMenu; import javax.swing.JScrollPane; import javax.swing.WindowConstants; -import java.awt.Color; -import java.awt.Desktop; -import java.awt.Dimension; -import java.awt.Toolkit; -import java.awt.event.ActionListener; -import java.io.File; -import java.io.IOException; -import java.lang.invoke.MethodHandles; -import java.util.ArrayList; -import java.util.List; -import java.util.function.ObjIntConsumer; + +import net.miginfocom.swing.MigLayout; + +import org.mastodon.mamut.clustering.util.HierarchicalClusteringResult; +import org.mastodon.mamut.model.Model; +import org.mastodon.model.tag.TagSetModel; +import org.mastodon.model.tag.TagSetStructure; +import org.mastodon.ui.util.ExtensionFileFilter; +import org.mastodon.ui.util.FileChooser; +import org.scijava.prefs.PrefService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A class that represents a UI view of a dendrogram.
* It encapsulates a {@link HierarchicalClusteringResult} object and a headline that write the parameters that were used for it. * @param the type of the objects that are clustered */ -public class DendrogramView< T > implements TagSetModel.TagSetModelListener +public class DendrogramView< T > extends JFrame implements TagSetModel.TagSetModelListener { private static final Logger logger = LoggerFactory.getLogger( MethodHandles.lookup().lookupClass() ); @@ -93,8 +97,6 @@ public class DendrogramView< T > implements TagSetModel.TagSetModelListener private final PrefService prefs; - private final JFrame frame; - private final JPanel canvas = new JPanel( new MigLayout( "fill" ) ); private final JCheckBox showThresholdCheckBox = new JCheckBox( "Show clustering threshold" ); @@ -126,21 +128,23 @@ public DendrogramView( final HierarchicalClusteringResult< T > hierarchicalClust final PrefService prefs, final String projectName ) { + super( "Hierarchical clustering of lineage trees" ); + this.hierarchicalClusteringResult = hierarchicalClusteringResult; this.model = model; this.prefs = prefs; this.projectName = projectName; - frame = new JFrame( "Hierarchical clustering of lineage trees" ); - frame.setDefaultCloseOperation( WindowConstants.DISPOSE_ON_CLOSE ); + int minHeight = 50; int initialDendrogramHeight = hierarchicalClusteringResult == null ? minHeight : ( hierarchicalClusteringResult.getObjectCount() ) * getDefaultFontHeight() + DendrogramPanel.DENDROGRAM_VERTICAL_OFFSET; initialDendrogramHeight += 200; initialDendrogramHeight = Math.min( initialDendrogramHeight, 1000 ); - frame.setSize( 1000, initialDendrogramHeight ); - frame.setLayout( new MigLayout( "insets 10, fill" ) ); - frame.add( canvas, "grow" ); + setSize( 1000, initialDendrogramHeight ); + setLayout( new MigLayout( "insets 10, fill" ) ); + add( canvas, "grow" ); + setDefaultCloseOperation( WindowConstants.DISPOSE_ON_CLOSE ); headlineLabel = new JLabel( headline ); dendrogramPanel = new DendrogramPanel<>( hierarchicalClusteringResult ); @@ -153,14 +157,6 @@ public DendrogramView( final HierarchicalClusteringResult< T > hierarchicalClust this.model.getTagSetModel().listeners().add( this ); } - /** - * Sets the visibility of the frame to {@code true}. - */ - public void show() - { - frame.setVisible( true ); - } - public JPanel getCanvas() { return canvas; @@ -214,8 +210,10 @@ private void initSettings() private void initBehavior() { - showThresholdCheckBox.addActionListener( ignore -> showThreshold( showThresholdCheckBox.isSelected() ) ); - showMedianCheckBox.addActionListener( ignore -> showMedian( showMedianCheckBox.isSelected() ) ); + ActionListener showThresholdListener = ignore -> showThreshold( showThresholdCheckBox.isSelected() ); + showThresholdCheckBox.addActionListener( showThresholdListener ); + ActionListener showMedianListener = ignore -> showMedian( showMedianCheckBox.isSelected() ); + showMedianCheckBox.addActionListener( showMedianListener ); ActionListener tagSetListener = event -> { selectedTagSet = tagSetComboBox.getSelectedItem() == null ? null : ( ( TagSetElement ) tagSetComboBox.getSelectedItem() ).tagSet; @@ -225,7 +223,24 @@ private void initBehavior() showTagLabelsCheckBox.addActionListener( tagSetListener ); tagSetComboBox.addActionListener( tagSetListener ); JPopupMenu popupMenu = new PopupMenu(); - menuButton.addActionListener( event -> popupMenu.show( menuButton, 0, menuButton.getHeight() ) ); + ActionListener menuListener = event -> popupMenu.show( menuButton, 0, menuButton.getHeight() ); + menuButton.addActionListener( menuListener ); + + addWindowListener( new WindowAdapter() + { + @Override + public void windowClosing( WindowEvent windowEvent ) + { + showThresholdCheckBox.removeActionListener( showThresholdListener ); + showMedianCheckBox.removeActionListener( showMedianListener ); + showRootLabelsCheckBox.removeActionListener( tagSetListener ); + showTagLabelsCheckBox.removeActionListener( tagSetListener ); + tagSetComboBox.removeActionListener( tagSetListener ); + menuButton.removeActionListener( menuListener ); + if ( null != model ) + model.getTagSetModel().listeners().remove( DendrogramView.this ); + } + } ); } private void showThreshold( final boolean showThreshold ) @@ -333,7 +348,7 @@ private PopupMenu() private void chooseFileAndExport( final String extension, final String fileName, final ObjIntConsumer< File > exportFunction ) { - File chosenFile = FileChooser.chooseFile( frame, fileName + '.' + extension, + File chosenFile = FileChooser.chooseFile( this, fileName + '.' + extension, new ExtensionFileFilter( extension ), "Save dendrogram to " + extension, FileChooser.DialogType.SAVE ); if ( chosenFile != null ) { From 73e991396f4f0a9610de9e3c30787f3500cc3555 Mon Sep 17 00:00:00 2001 From: Stefan Hahmann Date: Thu, 5 Dec 2024 12:18:05 +0100 Subject: [PATCH 2/4] Synchronize the branch graph before performing the clustering * Without this, the clustering may lead to wrong results, since it operates on a branch graph that potentially is outdated. --- .../org/mastodon/mamut/clustering/ClusterLineagesController.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java b/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java index 495919eb0..15dc480cb 100644 --- a/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java +++ b/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java @@ -155,6 +155,7 @@ public String createTagSet() private String runClassification() { + referenceProjectModel.getBranchGraphSync().sync(); ReentrantReadWriteLock.ReadLock lock = referenceModel.getGraph().getLock().readLock(); lock.lock(); String createdTagSetName; From 27eaf4f2f767a7ad65d5ec8b7338cb50be2b99c9 Mon Sep 17 00:00:00 2001 From: Stefan Hahmann Date: Thu, 5 Dec 2024 12:27:26 +0100 Subject: [PATCH 3/4] Rename some methods and classes from classify to cluster --- .../clustering/ClusterLineagesController.java | 43 +++++++++---------- ...leProject.java => ClusterableProject.java} | 6 +-- 2 files changed, 24 insertions(+), 25 deletions(-) rename src/main/java/org/mastodon/mamut/clustering/multiproject/{ClassifiableProject.java => ClusterableProject.java} (91%) diff --git a/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java b/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java index 15dc480cb..443ccba31 100644 --- a/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java +++ b/src/main/java/org/mastodon/mamut/clustering/ClusterLineagesController.java @@ -34,7 +34,7 @@ import org.mastodon.mamut.ProjectModel; import org.mastodon.mamut.clustering.config.ClusteringMethod; import org.mastodon.mamut.clustering.config.SimilarityMeasure; -import org.mastodon.mamut.clustering.multiproject.ClassifiableProject; +import org.mastodon.mamut.clustering.multiproject.ClusterableProject; import org.mastodon.mamut.clustering.multiproject.ExternalProjects; import org.mastodon.mamut.clustering.util.HierarchicalClusteringResult; import org.mastodon.mamut.clustering.config.CropCriteria; @@ -145,7 +145,7 @@ public String createTagSet() try { running = true; - return runClassification(); + return runClustering(); } finally { @@ -153,7 +153,7 @@ public String createTagSet() } } - private String runClassification() + private String runClustering() { referenceProjectModel.getBranchGraphSync().sync(); ReentrantReadWriteLock.ReadLock lock = referenceModel.getGraph().getLock().readLock(); @@ -161,16 +161,16 @@ private String runClassification() String createdTagSetName; try { - Pair< List< ClassifiableProject >, double[][] > rootsAndDistances = getRootsAndDistanceMatrix(); - List< ClassifiableProject > rootsMatrix = rootsAndDistances.getLeft(); + Pair< List< ClusterableProject >, double[][] > rootsAndDistances = getRootsAndDistanceMatrix(); + List< ClusterableProject > rootsMatrix = rootsAndDistances.getLeft(); double[][] distances = rootsAndDistances.getRight(); - ClassifiableProject referenceProject = rootsMatrix.get( 0 ); + ClusterableProject referenceProject = rootsMatrix.get( 0 ); HierarchicalClusteringResult< BranchSpotTree > hierarchicalClusteringResult = - classifyLineageTrees( referenceProject.getTrees(), distances ); + clusterLineageTrees( referenceProject.getTrees(), distances ); Function< BranchSpotTree, BranchSpot > branchSpotProvider = BranchSpotTree::getBranchSpot; - createdTagSetName = applyClassification( hierarchicalClusteringResult, referenceModel, branchSpotProvider ); + createdTagSetName = applyTagSet( hierarchicalClusteringResult, referenceModel, branchSpotProvider ); if ( addTagSetToExternalProjects && rootsMatrix.size() > 1 ) - classifyExternalProjects( rootsMatrix, distances ); + clusterExternalProjects( rootsMatrix, distances ); if ( showDendrogram ) showDendrogram( hierarchicalClusteringResult ); } @@ -181,21 +181,21 @@ private String runClassification() return createdTagSetName; } - private void classifyExternalProjects( final List< ClassifiableProject > rootsMatrix, final double[][] distances ) + private void clusterExternalProjects( final List< ClusterableProject > rootsMatrix, final double[][] distances ) { Function< BranchSpotTree, BranchSpot > branchSpotProvider; for ( int i = 1; i < rootsMatrix.size(); i++ ) // NB: start at 1 to skip reference project { - ClassifiableProject project = rootsMatrix.get( i ); + ClusterableProject project = rootsMatrix.get( i ); HierarchicalClusteringResult< BranchSpotTree > hierarchicalClusteringResult = - classifyLineageTrees( project.getTrees(), distances ); + clusterLineageTrees( project.getTrees(), distances ); ProjectModel projectModel = project.getProjectModel(); Model model = projectModel.getModel(); File file = project.getFile(); branchSpotProvider = branchSpotTree -> model.getBranchGraph().vertices().stream() .filter( ( branchSpot -> branchSpot.getFirstLabel().equals( branchSpotTree.getName() ) ) ) .findFirst().orElse( null ); - applyClassification( hierarchicalClusteringResult, model, branchSpotProvider ); + applyTagSet( hierarchicalClusteringResult, model, branchSpotProvider ); try { ProjectSaver.saveProject( file, projectModel ); @@ -208,10 +208,10 @@ private void classifyExternalProjects( final List< ClassifiableProject > rootsMa } } - private Pair< List< ClassifiableProject >, double[][] > getRootsAndDistanceMatrix() + private Pair< List< ClusterableProject >, double[][] > getRootsAndDistanceMatrix() { List< BranchSpotTree > roots = getRoots(); - ClassifiableProject referenceProject = new ClassifiableProject( null, referenceProjectModel, roots ); + ClusterableProject referenceProject = new ClusterableProject( null, referenceProjectModel, roots ); if ( externalProjects.isEmpty() ) { double[][] distances = HierarchicalClusteringUtils.getDistanceMatrix( roots, similarityMeasure ); @@ -219,7 +219,7 @@ private Pair< List< ClassifiableProject >, double[][] > getRootsAndDistanceMatri } List< String > commonRootNames = findCommonRootNames(); - List< ClassifiableProject > projects = new ArrayList<>(); + List< ClusterableProject > projects = new ArrayList<>(); keepCommonRootsAndSort( roots, commonRootNames ); projects.add( referenceProject ); @@ -227,9 +227,9 @@ private Pair< List< ClassifiableProject >, double[][] > getRootsAndDistanceMatri { List< BranchSpotTree > externalRoots = getRoots( project.getValue() ); keepCommonRootsAndSort( externalRoots, commonRootNames ); - projects.add( new ClassifiableProject( project.getKey(), project.getValue(), externalRoots ) ); + projects.add( new ClusterableProject( project.getKey(), project.getValue(), externalRoots ) ); } - List< List< BranchSpotTree > > treeMatrix = projects.stream().map( ClassifiableProject::getTrees ).collect( Collectors.toList() ); + List< List< BranchSpotTree > > treeMatrix = projects.stream().map( ClusterableProject::getTrees ).collect( Collectors.toList() ); return Pair.of( projects, HierarchicalClusteringUtils.getAverageDistanceMatrix( treeMatrix, similarityMeasure ) ); } @@ -290,7 +290,7 @@ private void showDendrogram( final HierarchicalClusteringResult< BranchSpotTree dendrogramView.setVisible( true ); } - private HierarchicalClusteringResult< BranchSpotTree > classifyLineageTrees( final List< BranchSpotTree > roots, + private HierarchicalClusteringResult< BranchSpotTree > clusterLineageTrees( final List< BranchSpotTree > roots, final double[][] distances ) { if ( roots.size() != distances.length ) @@ -307,9 +307,8 @@ private HierarchicalClusteringResult< BranchSpotTree > classifyLineageTrees( fin return result; } - private String applyClassification( final HierarchicalClusteringResult< BranchSpotTree > hierarchicalClusteringResult, - final Model model, - final Function< BranchSpotTree, BranchSpot > branchSpotProvider ) + private String applyTagSet( final HierarchicalClusteringResult< BranchSpotTree > hierarchicalClusteringResult, + final Model model, final Function< BranchSpotTree, BranchSpot > branchSpotProvider ) { String tagSetName = getTagSetName(); List< HierarchicalClusteringResult.Group< BranchSpotTree > > groups = hierarchicalClusteringResult.getGroups(); diff --git a/src/main/java/org/mastodon/mamut/clustering/multiproject/ClassifiableProject.java b/src/main/java/org/mastodon/mamut/clustering/multiproject/ClusterableProject.java similarity index 91% rename from src/main/java/org/mastodon/mamut/clustering/multiproject/ClassifiableProject.java rename to src/main/java/org/mastodon/mamut/clustering/multiproject/ClusterableProject.java index 2b081cda1..4b9247088 100644 --- a/src/main/java/org/mastodon/mamut/clustering/multiproject/ClassifiableProject.java +++ b/src/main/java/org/mastodon/mamut/clustering/multiproject/ClusterableProject.java @@ -35,9 +35,9 @@ import java.util.List; /** - * Small helper class to hold a project file, its {@link ProjectModel} and the trees that are to be classified. + * Small helper class to hold a project file, its {@link ProjectModel} and the trees that are to be clustered. */ -public class ClassifiableProject +public class ClusterableProject { private final File file; @@ -45,7 +45,7 @@ public class ClassifiableProject private final List< BranchSpotTree > trees; - public ClassifiableProject( final File file, final ProjectModel projectModel, final List< BranchSpotTree > trees ) + public ClusterableProject( final File file, final ProjectModel projectModel, final List< BranchSpotTree > trees ) { this.file = file; this.projectModel = projectModel; From 32d8d5d498436dd764d64381b1cae2be10fb0262 Mon Sep 17 00:00:00 2001 From: Stefan Hahmann Date: Thu, 5 Dec 2024 12:28:46 +0100 Subject: [PATCH 4/4] Add temporary workaround for a bug in mastodon-core --- .../treesimilarity/tree/BranchSpotTree.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/mastodon/mamut/clustering/treesimilarity/tree/BranchSpotTree.java b/src/main/java/org/mastodon/mamut/clustering/treesimilarity/tree/BranchSpotTree.java index ae2d51415..bf8e9aca9 100644 --- a/src/main/java/org/mastodon/mamut/clustering/treesimilarity/tree/BranchSpotTree.java +++ b/src/main/java/org/mastodon/mamut/clustering/treesimilarity/tree/BranchSpotTree.java @@ -169,7 +169,16 @@ public void setParams( final boolean includeName, final boolean includeTag, fina private String getTagLabel() { Spot ref = model.getGraph().vertexRef(); - String tagLabel = LegacyTagSetUtils.getTagLabel( model, branchSpot, tagSet, ref ); + String tagLabel = null; + try + { + // TODO: this try-catch block is a workaround. It will not be needed anymore after mastodon-core beta-34 + tagLabel = LegacyTagSetUtils.getTagLabel( model, branchSpot, tagSet, ref ); + } + catch ( NullPointerException e ) + { + // happens, when the branchSpot is not in the model anymore + } model.getGraph().releaseRef( ref ); if ( tagLabel == null ) tagLabel = "";