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 compressed segmentation #6947

Merged
merged 19 commits into from
Apr 12, 2023
Merged

Implement compressed segmentation #6947

merged 19 commits into from
Apr 12, 2023

Conversation

frcroth
Copy link
Member

@frcroth frcroth commented Mar 28, 2023

URL of deployed dev instance (used for testing):

Steps to test:

  • Explore and load a segmentation
  • gs://fafb-ffn1-20200412/segmentation (compare with neuroglancer)
  • gs://neuroglancer-janelia-flyem-hemibrain/v1.2/segmentation
    I did not find segmentation datasets with uint32 as data type

Current problems:

  • Segment ids are not correct. That is, the shapes that result are correct, but they do not have the right ids. Also, some segments are missing.
  • Performance is pretty bad right now
  • Ensure License compliance
  • Update docs: Remove wkconnect?

Issues:


(Please delete unneeded items, merge only when none are left open)

@frcroth frcroth changed the title WIP: Implement compressed segmentation Implement compressed segmentation Apr 3, 2023
@frcroth frcroth marked this pull request as ready for review April 3, 2023 13:04
@frcroth frcroth requested a review from fm3 April 3, 2023 13:04
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool stuff :)


private def decompressChannel(input: Array[Int], volumeSize: Array[Int], blockSize: Array[Int]): Array[T] = {
assert(blockSize.length == 3)
val numElements = volumeSize(0) * volumeSize(1) * volumeSize(2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the order of volumeSize elements fixed? can we rely on 0,1,2 being x,y,z, and 3 being c? Could there be a time axis as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specification only describes 3d volume data, so I think it can be left like this.

@normanrz
Copy link
Member

normanrz commented Apr 6, 2023

Probably a different issue: I just tried to import gs://fafb-ffn1-20200412/segmentation and then gs://neuroglancer-fafb-data/fafb_v14/fafb_v14_clahe. The explore feature did not pick up that the clahe layer actually started in mag 2 (8x8x40nm).

@fm3
Copy link
Member

fm3 commented Apr 6, 2023

Probably a different issue: I just tried to import gs://fafb-ffn1-20200412/segmentation and then gs://neuroglancer-fafb-data/fafb_v14/fafb_v14_clahe. The explore feature did not pick up that the clahe layer actually started in mag 2 (8x8x40nm).

Yes, that’s #6821

@frcroth frcroth requested a review from fm3 April 12, 2023 11:05
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the feedback! I added just one more small suggestion


override def toString: String = s"compressor=$getId/dataType=${dataType.toString}"

private def blockSizeVec = Vec3Int(x = blockSize(0), y = blockSize(1), z = blockSize(2))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could blockSize be Vec3Int everywhere? (even in case class PrecomputedScale). Then the json validator would already reject it if it does not match, and we wouldn’t need the conversion here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I did not know Vec3Int could be automatically parsed.

@frcroth frcroth requested a review from fm3 April 12, 2023 13:37
Copy link
Member

@fm3 fm3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 🚀

@frcroth frcroth merged commit 4091967 into master Apr 12, 2023
@frcroth frcroth deleted the compressed-segmentation branch April 12, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support compressed segmentation for Neuroglancer precomputed datasets
3 participants