feature/thomas-intensity-changes-b - #243
Conversation
…separate BeamCorrectionParameters class that can be more easily reused by spark pipelines
…xlog zarr data from Google cloud storage
…ient and BeamCorrectionSparkClient runs
…CorrectionType parameter with possible values: OVERWRITE_SOURCE, WRITE_TO_TARGET, or SKIP
…sWithDifferentMFOVs method to removeMatchPairsThatReferenceTilesOutsideThisCollection
…service since we generate that match data in an earlier intensity correction step. The MFOVAsTileParameters deriveSfovMatchData field determines whether matches are simply read or are generated during the pre-alignment process. In "small" tests using 5 Google nodes with two w61_s140_r00 z layers, the run with deriveSfovMatchData=false took 61 minutes and the run with deriveSfovMatchData=true took 79 minutes.
…bute name consistent
…ive. This makes it a little clearer that you are excluding cross MFOV match pair patching when you specify a negative weight (rather than leaving the weight unspecified).
…lated MFOV patching is not done
…hat wasted compute time (e.g. 15 minutes of a 90-minute run)
minnerbe
left a comment
minnerbe
left a comment
There was a problem hiding this comment.
Thanks for putting this together, @trautmane! I have skimmed the code and only dug into a few targeted spots. All looks good to me. As always, my review comments are mostly just suggestions.
| if (pTileSpec == null) { | ||
| if (qTileSpec == null) { | ||
| sb.append("s ").append(pair.getpId()).append(" and ").append(pair.getqId()); | ||
| } | ||
| sb.append(" ").append(pair.getpId()); | ||
| } else { | ||
| sb.append(" ").append(pair.getqId()); | ||
| } |
There was a problem hiding this comment.
| if (pTileSpec == null) { | |
| if (qTileSpec == null) { | |
| sb.append("s ").append(pair.getpId()).append(" and ").append(pair.getqId()); | |
| } | |
| sb.append(" ").append(pair.getpId()); | |
| } else { | |
| sb.append(" ").append(pair.getqId()); | |
| } | |
| if (qTileSpec == null) { | |
| if (pTileSpec == null) { | |
| sb.append("s ").append(pair.getpId()).append(" and "); | |
| } | |
| sb.append(" ").append(pair.getqId()); | |
| } else { | |
| sb.append(" ").append(pair.getpId()); | |
| } |
There was a problem hiding this comment.
In the current formulation, pId is printed twice if both tile specs are null. This change should print p, q, or s p and q` correctly, depending on the situation.
|
|
||
| if (excludeSameMfovNeighbors && (pMfov.equals(MultiSemUtilities.getMagcMfovForTileId(qTileId)))) { | ||
| continue; | ||
| } else if (excludeDifferentMfovNeighbors && (! pMfov.equals(MultiSemUtilities.getMagcMfovForTileId(qTileId)))) { |
There was a problem hiding this comment.
For readability, consider declaring final boolean isSameMfov = pMfov.equals(MultiSemUtilities.getMagcMfovForTileId(qTileId)).
| * | ||
| * @return the parsed resource or null if it is not present in the container. | ||
| */ | ||
| private static JsonObject readDatasetJson(final ZarrKeyValueReader reader, |
There was a problem hiding this comment.
Just a general comment on this file: I've had to read the metadata as plain json instead of going through N5's key-value reader because it cannot deal with some of the values that are written by python. This is fixed in a newer N5 version, which is pulled in in #239. This file is also significantly simplified there.
| * Spark client for updating the tileId for all tiles in the stack | ||
| * to use the new wafer 60 and 61 render order. | ||
| */ | ||
| public class TileIdHackClient |
There was a problem hiding this comment.
I think I'd prefer something like TileIdUpdateClient or ChangeTileIdClient. "Hack" sounds like we're doing something wrong.. ;-)
2 participants
This pull request involves changing tile identifiers to improve render order, adding beam intensity correction as a spark pipeline step, and persisting/reusing within-MFOV match data during beam intensity correction and MFOV-as-tile processing.