Hic track #1806

jrobinso · 2026-01-09T04:36:11Z

No description provided.

Other optimzations

Copilot

Pull request overview

This PR introduces a dedicated HicInteractionTrack subclass to separate HiC-specific behavior from the generic InteractionTrack implementation. The refactoring replaces conditional logic based on the isHIC flag with proper object-oriented inheritance.

Key changes:

New HicInteractionTrack subclass handles HiC-specific features (normalization options, contact maps, specialized menus)
ContactRecord now stores both raw counts and normalized counts
Reservoir sampling added to WrappedInteractionSource for feature count limiting
Contact map view UI improvements including throttled slider updates, bin size display, and tooltips

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
TrackLoader.java	Routes HiC format files to HicInteractionTrack, other interaction files to InteractionTrack
IGVSessionReader.java	Adds HicInteractionTrack to session restoration logic
HicFile.java	Updates to use new ContactRecord with normCounts field
ContactRecord.java	Extends record to include both counts and normCounts fields with convenience constructor
WrappedInteractionSource.java	Implements reservoir sampling for maxFeatureCount limiting
NestedArcRenderer.java	Simplifies color handling with default track color and consistent transparency
InteractionTrack.java	Removes isHIC conditional logic, adds extensibility hooks for subclasses
HicSource.java	Applies normalization at query time, uses normCounts throughout
HicInteractionTrack.java	New subclass implementing HiC-specific behavior (zoom filtering, normalization menu, contact map)
ContactMapView.java	Adds tooltip display, throttled slider updates, bin size label, and improved color scale management

Comments suppressed due to low confidence (1)

src/main/java/org/igv/bedpe/HicSource.java:175

The adjacent region queries on lines 170 and 175 are using "NONE" for normalization, while the main region query on line 164 uses the requested normalization. This creates inconsistency where diagonal contacts will be normalized but adjacent region contacts will not be normalized. All three queries should use the same normalization parameter to ensure consistent data processing.

            List<ContactRecord> adjacentRecords = hicFile.getContactRecords(region1, adjacent, "BP", binSize, "NONE", false);
            records.addAll(adjacentRecords);
        }

        Region adjacent2 = new Region(chr, end, end + (end - start));
        List<ContactRecord> adjacentRecords2 = hicFile.getContactRecords(region1, adjacent2, "BP", binSize, "NONE", false);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-09T04:40:43Z

src/main/java/org/igv/bedpe/ContactMapView.java

+                    ContactRecord found = cachedRecords.stream()
+                            .filter(r -> (r.bin1() == searchBin1 && r.bin2() == searchBin2) ||
+                                    (r.bin1() == searchBin2 && r.bin2() == searchBin1))
+                            .findFirst()
+                            .orElse(null);


This tooltip lookup performs a linear search through cachedRecords on every mouse movement, which could be inefficient for large datasets. Consider indexing cachedRecords by bin pairs in a HashMap for O(1) lookup performance instead of streaming through the entire list on each mouseMoved event.

Copilot · 2026-01-09T04:40:43Z

src/main/java/org/igv/bedpe/ContactMapView.java


            if (x >= 0 && y >= 0) {
-                Color color = colorScale.getColor(record.counts());
+                Color color = colorScale.getColor(record.normCounts());


Confusing name: method renderMapFromCache also refers to field color (without qualifying it with 'this').

Other optimzations

Disable map tooltext -- very costly with current design

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 14 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-09T06:23:59Z

src/main/java/org/igv/hic/HicFile.java


            for (ContactRecord rec : block.records) {
-                if (allRecords || (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2) && rec.counts() > 1) {
+                if ((allRecords || (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2)) && rec.counts() > countsTreshold) {


There's a spelling error in the parameter name. It should be "countsThreshold" not "countsTreshold" (missing 'h' in 'threshold').

src/main/java/org/igv/bedpe/WrappedInteractionSource.java

Copilot · 2026-01-09T06:24:00Z

src/main/java/org/igv/bedpe/InteractionTrack.java


+    /**
+     * Hook method for subclasses to filter features based on zoom level.
+     * Default implementation returns features unchanged.


The documentation for the hook method doesn't include @param or @return tags. For consistency with Java documentation standards, consider adding "@param features The features to filter", "@param interval The loaded interval", "@param referenceFrame The reference frame", and "@return The filtered list of features" to make the method's contract clearer.

Suggested change

* Default implementation returns features unchanged.

* Default implementation returns features unchanged.

*

* @param features The features to filter

* @param interval The loaded interval

* @param referenceFrame The reference frame

* @return The filtered list of features

src/main/java/org/igv/bedpe/InteractionTrack.java

Copilot · 2026-01-09T06:24:01Z

src/main/java/org/igv/bedpe/ContactMapView.java

     * Also updates the slider range based on the min/max of filtered counts.
     * Does NOT update the slider position - caller should do that after updating the color scale.
     *
     * @return the percentile value, or -1 if contactRecords is empty


The JavaDoc @return tag says "the percentile value, or -1 if contactRecords is empty" but the implementation now returns 10 when contactRecords is empty (line 839). Update the documentation to match the actual return value.

Suggested change

* @return the percentile value, or -1 if contactRecords is empty

* @return the percentile value, or 10 if contactRecords is empty

src/main/java/org/igv/bedpe/HicInteractionTrack.java

Copilot · 2026-01-09T06:24:02Z

src/main/java/org/igv/hic/HicFile.java


            for (ContactRecord rec : block.records) {
-                if (allRecords || (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2) && rec.counts() > 1) {
+                if ((allRecords || (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2)) && rec.counts() > countsTreshold) {


The threshold comparison uses strict greater than (>) which means values equal to the threshold are excluded. However, the parameter name "countsTreshold" (which should be "countsThreshold") suggests this is a minimum threshold. Consider whether this should be >= instead of > for more intuitive threshold behavior, where a threshold of 1 would include records with counts of 1 or more.

Suggested change

if ((allRecords || (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2)) && rec.counts() > countsTreshold) {

if ((allRecords || (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2)) && rec.counts() >= countsTreshold) {

Copilot · 2026-01-09T06:24:02Z

src/main/java/org/igv/bedpe/WrappedInteractionSource.java

    @Override
    public List<BedPE> getFeatures(String chr, int start, int end, double bpPerPixel, String normalization, int maxFeatureCount) throws IOException {
-        Iterator<BedPE>  featureIterator = featureSource.getFeatures(chr, start, end);
-        List<BedPE> list = new ArrayList<>();
-        while (featureIterator != null && featureIterator.hasNext()) {
-            list.add(featureIterator.next());
+        Iterator<BedPE> featureIterator = featureSource.getFeatures(chr, start, end);
+        List<BedPE> reservoir = new ArrayList<>();
+
+        if (featureIterator == null) {
+            return reservoir;
+        }
+
+        // If maxFeatureCount <= 0 treat as "no limit" and return all features
+        if (maxFeatureCount <= 0) {
+            while (featureIterator.hasNext()) {
+                reservoir.add(featureIterator.next());
+            }
+            return reservoir;
        }
-        return list;
+
+        int i = 0; // number of items seen so far
+
+        while (featureIterator.hasNext()) {
+            BedPE item = featureIterator.next();
+            if (i < maxFeatureCount) {
+                // fill the reservoir
+                reservoir.add(item);
+            } else {
+                // replace elements with decreasing probability
+                int r = ThreadLocalRandom.current().nextInt(i + 1); // 0..i
+                if (r < maxFeatureCount) {
+                    reservoir.set(r, item);
+                }
+            }
+            i++;
+        }
+
+        return reservoir;
    }


The reservoir sampling implementation in WrappedInteractionSource adds significant new logic for limiting feature counts. While there's an existing test for HicSource.getFeatures() that verifies the maxFeatureCount parameter works, there are no tests specifically for the reservoir sampling algorithm in WrappedInteractionSource. Consider adding tests to verify: 1) that when maxFeatureCount <= 0, all features are returned, 2) that when there are fewer features than maxFeatureCount, all are returned, 3) that when there are more features than maxFeatureCount, exactly maxFeatureCount features are returned, and 4) that the reservoir sampling produces a statistically uniform distribution over multiple runs.

Copilot · 2026-01-09T06:24:03Z

src/main/java/org/igv/hic/HicFile.java

+                                                 int binSize,
+                                                 String normalization,
+                                                 boolean allRecords,
+                                                 int countsTreshold) throws IOException {


There's a spelling error in the parameter name. It should be "countsThreshold" not "countsTreshold" (missing 'h' in 'threshold').

Copilot · 2026-01-09T06:24:03Z

src/main/java/org/igv/bedpe/ContactMapView.java

+        int actualBinSize = binSize;
+        if (frame.getChrName().equalsIgnoreCase("all")) {
+            actualBinSize *= 1000;


When frame.getChrName() is "all", the actualBinSize is multiplied by 1000 (line 795). If binSize is already close to Integer.MAX_VALUE / 1000 (approximately 2.1 million), this could cause integer overflow. Consider using a long for actualBinSize to avoid potential overflow issues, or document the valid range for binSize values.

jrobinso added 2 commits January 8, 2026 18:29

minor optimizations of hic track and contact map view

6e2bd72

Refactor InteractionTrack to eliminate "isHIC" checks

30eb974

Other optimzations

jrobinso requested a review from Copilot January 9, 2026 04:36

Copilot started reviewing on behalf of jrobinso January 9, 2026 04:36 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

jrobinso added 3 commits January 8, 2026 21:13

Refactor InteractionTrack to eliminate "isHIC" checks

8ed0b66

Other optimzations

...

b9f22ac

Fixes for whole genome view

9d81ce2

Disable map tooltext -- very costly with current design

jrobinso requested a review from Copilot January 9, 2026 06:17

Copilot started reviewing on behalf of jrobinso January 9, 2026 06:17 View session

Copilot AI reviewed Jan 9, 2026

View reviewed changes

fix typo, unit test

e6d0baf

jrobinso merged commit 8ae555e into main Jan 9, 2026
2 checks passed

jrobinso deleted the hic_track branch January 9, 2026 06:48

-     * Default implementation returns features unchanged.
+     * Default implementation returns features unchanged.
+     *
+     * @param features        The features to filter
+     * @param interval        The loaded interval
+     * @param referenceFrame  The reference frame
+     * @return The filtered list of features

	* @return the percentile value, or -1 if contactRecords is empty
	* @return the percentile value, or 10 if contactRecords is empty

	if ((allRecords \|\| (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2)) && rec.counts() > countsTreshold) {
	if ((allRecords \|\| (rec.bin1() >= x1 && rec.bin1() < x2 && rec.bin2() >= y1 && rec.bin2() < y2)) && rec.counts() >= countsTreshold) {

Hic track #1806

Hic track #1806

Uh oh!

Conversation

jrobinso commented Jan 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants