-
Notifications
You must be signed in to change notification settings - Fork 242
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.
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
Proposed replacement for progress PR #1302
Changes from 10 commits
ca0e776
6645b9b
d9beceb
4ad0ffe
8419368
ba3ec0f
0829c96
de71c3b
4a23342
49cb51f
cea9932
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,10 +18,14 @@ abstract public class AbstractProgressLogger implements ProgressLoggerInterface | |
private long startTime; | ||
private final NumberFormat fmt = new DecimalFormat("#,###"); | ||
private final NumberFormat timeFmt = new DecimalFormat("00"); | ||
private long processed; | ||
private long lastStartTime; | ||
private String lastChrom; | ||
private int lastPos; | ||
private long processed = 0; | ||
// Set to -1 until the first record is added | ||
private long lastStartTime = -1; | ||
private String lastChrom = null; | ||
private int lastPos = 0; | ||
private String lastReadName = null; | ||
private long countNonIncreasing = 0; | ||
final private long PRINT_READ_NAME_THRESHOLD = 1000; | ||
|
||
/** | ||
* Construct an AbstractProgressLogger. | ||
|
@@ -60,18 +64,26 @@ private synchronized void record() { | |
if (this.lastChrom == null) readInfo = "*/*"; | ||
else readInfo = this.lastChrom + ":" + fmt.format(this.lastPos); | ||
|
||
final String rnInfo; | ||
|
||
if (lastReadName != null && countNonIncreasing > PRINT_READ_NAME_THRESHOLD) { | ||
rnInfo = ". Last read name: " + lastReadName; | ||
} else { | ||
rnInfo = ""; | ||
} | ||
|
||
final long n = (this.processed % this.n == 0) ? this.n : this.processed % this.n; | ||
|
||
log(this.verb, " ", processed, " " + noun + ". Elapsed time: ", elapsed, "s. Time for last ", fmt.format(n), | ||
": ", period, "s. Last read position: ", readInfo); | ||
": ", period, "s. Last read position: ", readInfo, rnInfo); | ||
} | ||
|
||
/** | ||
* Logs the last last record if it wasn't previously logged. | ||
* @return boolean true if logging was triggered, false otherwise | ||
*/ | ||
public synchronized boolean log() { | ||
if (this.processed % this.n != 0) { | ||
if (processed % this.n != 0) { | ||
record(); | ||
return true; | ||
} | ||
|
@@ -80,41 +92,53 @@ public synchronized boolean log() { | |
} | ||
} | ||
|
||
@Override | ||
public synchronized boolean record(final String chrom, final int pos) { | ||
this.lastChrom = chrom; | ||
this.lastPos = pos; | ||
if (this.lastStartTime == -1) { | ||
this.lastStartTime = System.currentTimeMillis(); | ||
protected synchronized boolean record(final String chrom, final int pos, final String rname) { | ||
if (chrom != null && chrom.equals(lastChrom) && pos < lastPos) { | ||
countNonIncreasing++; | ||
if (countNonIncreasing == PRINT_READ_NAME_THRESHOLD) { | ||
log("Seen many non-increasing record positions. Printing Read-names as well."); | ||
} | ||
} else { | ||
lastChrom = chrom; | ||
} | ||
lastPos = pos; | ||
lastReadName = rname; | ||
if (lastStartTime == -1) { | ||
lastStartTime = System.currentTimeMillis(); | ||
} | ||
if (++this.processed % this.n == 0) { | ||
if (++processed % n == 0) { | ||
record(); | ||
return true; | ||
} | ||
else { | ||
} else { | ||
return false; | ||
} | ||
} | ||
|
||
@Override | ||
public synchronized boolean record(final String chrom, final int pos) { | ||
return record(chrom, pos, null); | ||
} | ||
|
||
/** | ||
* Records that a given record has been processed and triggers logging if necessary. | ||
* @return boolean true if logging was triggered, false otherwise | ||
*/ | ||
@Override | ||
public synchronized boolean record(final SAMRecord rec) { | ||
if (SAMRecord.NO_ALIGNMENT_REFERENCE_NAME.equals(rec.getReferenceName())) { | ||
return record(null, 0); | ||
} | ||
else { | ||
return record(rec.getReferenceName(), rec.getAlignmentStart()); | ||
return record(null, 0, rec.getReadName()); | ||
} else { | ||
return record(rec.getReferenceName(), rec.getAlignmentStart(), rec.getReadName()); | ||
} | ||
} | ||
|
||
/** Records multiple SAMRecords and triggers logging if necessary. */ | ||
@Override | ||
public boolean record(final SAMRecord... recs) { | ||
boolean triggered = false; | ||
for (final SAMRecord rec : recs) triggered = record(rec) || triggered; | ||
for (final SAMRecord rec : recs) { | ||
triggered = record(rec) || triggered; | ||
} | ||
return triggered; | ||
} | ||
|
||
|
@@ -125,22 +149,25 @@ public boolean record(final SAMRecord... recs) { | |
public long getElapsedSeconds() { return (System.currentTimeMillis() - this.startTime) / 1000; } | ||
|
||
/** Resets the start time to now and the number of records to zero. */ | ||
public void reset() { | ||
public synchronized void reset() { | ||
startTime = System.currentTimeMillis(); | ||
processed = 0; | ||
// Set to -1 until the first record is added | ||
lastStartTime = -1; | ||
lastChrom = null; | ||
lastPos = 0; | ||
lastReadName = null; | ||
countNonIncreasing = 0; | ||
} | ||
|
||
/** Left pads a string until it is at least the given length. */ | ||
private String pad (String in, final int length) { | ||
while (in.length() < length) { | ||
in = " " + in; | ||
protected static String pad(final String in, final int length) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make this default package instead of protected. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
final StringBuilder inBuilder = new StringBuilder(Math.max(length, in.length())); | ||
while (inBuilder.length() < length - in.length()) { | ||
inBuilder.append(" "); | ||
} | ||
|
||
return in; | ||
return inBuilder.append(in).toString(); | ||
} | ||
|
||
/** Formats a number of seconds into hours:minutes:seconds. */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package htsjdk.samtools.util | ||
|
||
import htsjdk.UnitSpec | ||
|
||
class AbstractProgressLoggerTest extends UnitSpec { | ||
|
||
"AbstractProgressLoggerTest.pad" should "pad the right amount of spaces in " in { | ||
Seq( | ||
("hello", 10, " hello"), | ||
("hello", 6, " hello"), | ||
("hello", 5, "hello"), | ||
("hello", 4, "hello"), | ||
("hello", -1, "hello")).foreach { case (in, len, expected) => | ||
AbstractProgressLogger.pad(in, len) shouldEqual expected | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The formatting seems really weird here, but maybe that's scala standard? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah.that's the default intellij formatting |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might as well make this static
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure