Skip to content
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

Implement model versioning #113

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/qupath/ext/instanseg/core/InstanSeg.java
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ private InstanSegResults runInstanSeg(ImageData<BufferedImage> imageData, Collec
.postProcess(postProcessor)
.downsample(downsample)
.build();

processor.processObjects(taskRunner, imageData, pathObjects);
int nObjects = pathObjects.stream().mapToInt(PathObject::nChildObjects).sum();
if (predictionProcessor instanceof TilePredictionProcessor tileProcessor) {
Expand Down
56 changes: 32 additions & 24 deletions src/main/java/qupath/ext/instanseg/core/InstanSegModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
public class InstanSegModel {

private static final Logger logger = LoggerFactory.getLogger(InstanSegModel.class);
private String version;
private URL modelURL = null;

/**
Expand All @@ -44,11 +45,13 @@ public class InstanSegModel {
private InstanSegModel(BioimageIoSpec.BioimageIoModel bioimageIoModel) {
this.model = bioimageIoModel;
this.path = Paths.get(model.getBaseURI());
this.version = model.getVersion();
this.name = model.getName();
}

private InstanSegModel(String name, URL modelURL) {
private InstanSegModel(String name, String version, URL modelURL) {
this.name = name;
this.version = version;
this.modelURL = modelURL;
}

Expand All @@ -68,30 +71,29 @@ public static InstanSegModel fromPath(Path path) throws IOException {
* @param browserDownloadUrl The download URL from eg GitHub
* @return A handle on the created model
*/
public static InstanSegModel fromURL(String name, URL browserDownloadUrl) {
return new InstanSegModel(name, browserDownloadUrl);
public static InstanSegModel fromURL(String name, String version, URL browserDownloadUrl) {
return new InstanSegModel(name, version, browserDownloadUrl);
}

/**
* Check if the model has been downloaded already.
* @return True if a flag has been set.
* @return True if the model has a known path that exists and is valid, or if a suitable directory can be found in the localModelPath
*/
public boolean isDownloaded(Path localModelPath) {
// Check path first - *sometimes* the model might be downloaded, but have a name
// that doesn't match with the filename (although we'd prefer this didn't happen...)
if (path != null && model != null && Files.exists(path))
if (path != null && model != null && isValidModel(path))
return true;
// todo: this should also check if the contents are what we expect
if (Files.exists(localModelPath.resolve(name))) {
try {
download(localModelPath);
} catch (IOException e) {
logger.error("Model directory exists but is not valid", e);
}
} else {
if (!Files.exists(localModelPath.resolve(name).resolve(version))) {
// The model may have been deleted or renamed - we won't be able to load it
return false;
}

try {
download(localModelPath);
} catch (IOException e) {
logger.error("Model directory exists but is not valid", e);
}
return path != null && model != null;
}

Expand All @@ -100,14 +102,16 @@ public boolean isDownloaded(Path localModelPath) {
* @throws IOException If an error occurs when downloading, unzipping, etc.
*/
public void download(Path localModelPath) throws IOException {
if (path != null && Files.exists(path) && model != null)
if (path != null && isValidModel(path) && model != null) {
return;
}
var zipFile = downloadZipIfNeeded(
this.modelURL,
localModelPath,
name);
name + "-" + version);
this.path = unzipIfNeeded(zipFile);
this.model = BioimageIoSpec.parseModel(path.toFile());
this.version = model.getVersion();
}

/**
Expand Down Expand Up @@ -213,8 +217,8 @@ public Optional<Path> getPath() {
@Override
public String toString() {
String name = getName();
String parent = getPath().map(Path::getFileName).map(Path::toString).orElse(null);
String version = getModel().map(BioimageIoSpec.BioimageIoModel::getVersion).orElse(null);
String parent = getPath().map(Path::getParent).map(Path::getFileName).map(Path::toString).orElse(null);
String version = getModel().map(BioimageIoSpec.BioimageIoModel::getVersion).orElse(this.version);
if (parent != null && !parent.equals(name)) {
name = parent + "/" + name;
}
Expand Down Expand Up @@ -290,16 +294,20 @@ private static boolean isDownloadedAlready(Path zipFile) {
return Files.exists(zipFile);
}

private static Path unzipIfNeeded(Path zipFile) throws IOException {
var outdir = zipFile.resolveSibling(zipFile.getFileName().toString().replace(".zip", ""));
private Path unzipIfNeeded(Path zipFile) throws IOException {
var zipSpec = BioimageIoSpec.parseModel(zipFile);
String version = zipSpec.getVersion();
var outdir = zipFile.resolveSibling(zipSpec.getName()).resolve(version);
if (!isUnpackedAlready(outdir)) {
try {
unzip(zipFile, zipFile.getParent());
unzip(zipFile, outdir);
// Files.delete(zipFile);
} catch (IOException e) {
logger.error("Error unzipping model", e);
// clean up files just in case!
Files.deleteIfExists(zipFile);
Files.deleteIfExists(outdir);
} finally {
Files.deleteIfExists(zipFile);
}
}
return outdir;
Expand All @@ -311,7 +319,7 @@ private static boolean isUnpackedAlready(Path outdir) {

private static void unzip(Path zipFile, Path destination) throws IOException {
if (!Files.exists(destination)) {
Files.createDirectory(destination);
Files.createDirectories(destination);
}
ZipInputStream zipIn = new ZipInputStream(new BufferedInputStream(new FileInputStream(zipFile.toFile())));
ZipEntry entry = zipIn.getNextEntry();
Expand Down Expand Up @@ -358,8 +366,8 @@ private Optional<Map<String, Double>> getPixelSize() {
var config = model.getConfig().getOrDefault("qupath", null);
if (config instanceof Map configMap) {
var axes = (List) configMap.get("axes");
String x = (String) ((Map) (axes.get(0))).get("step");
String y = (String) ((Map) (axes.get(1))).get("step");
String x = String.valueOf(((Map) (axes.get(0))).get("step"));
String y = String.valueOf(((Map) (axes.get(1))).get("step"));
return Optional.of(Map.of(
"x", Double.valueOf(x),
"y", Double.valueOf(y)
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/qupath/ext/instanseg/ui/GitHubUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public String toString() {
static List<GitHubRelease> getReleases(Path modelDir) {
Path cachedReleases = modelDir == null ? null : modelDir.resolve("releases.json");

String uString = "https://api.github.com/repos/instanseg/InstanSeg/releases";
String uString = "https://api.github.com/repos/alanocallaghan/InstanSeg/releases";
HttpRequest request = HttpRequest.newBuilder()
.uri(URI.create(uString))
.GET()
Expand Down
21 changes: 13 additions & 8 deletions src/main/java/qupath/ext/instanseg/ui/InstanSegController.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -549,6 +550,7 @@ private InstanSegModel downloadModel(InstanSegModel model, Path modelDir) {
});
} catch (IOException ex) {
Dialogs.showErrorNotification(resources.getString("title"), resources.getString("error.downloading"));
logger.error("Error download model", ex);
}
return model;
}
Expand Down Expand Up @@ -618,15 +620,18 @@ private static List<InstanSegModel> getRemoteModels() {
logger.info("No releases found.");
return List.of();
}
var release = releases.getFirst();
var assets = GitHubUtils.getAssets(release);
List<InstanSegModel> models = new ArrayList<>();
for (var asset : assets) {
models.add(
InstanSegModel.fromURL(
asset.getName().replace(".zip", ""),
asset.getUrl())
);
for (var release: releases) {
var assets = GitHubUtils.getAssets(release);
for (var asset: assets) {
models.add(
InstanSegModel.fromURL(
asset.getName().replace(".zip", ""),
release.getName(),
asset.getUrl()
)
);
}
}
return List.copyOf(models);
}
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/qupath/ext/instanseg/ui/Watcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,11 @@ private void handleModelDirectoryChange(ObservableValue<? extends Path> observab
// Currently, we look *only* in the model directory for models
// But we could register subdirectories here if we wanted (e.g. 'local', 'downloaded')
if (oldPath != null) {
unregister(oldPath);
unregister(oldPath.resolve("local"));
}
if (newPath != null && Files.isDirectory(newPath)) {
if (newPath != null && Files.isDirectory(newPath.resolve("local"))) {
try {
register(newPath);
register(newPath.resolve("local"));
} catch (IOException e) {
logger.error("Unable to register new model directory", e);
}
Expand Down