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

Improve speed and API of local thickness #896

Open
5 tasks
jgostick opened this issue Oct 13, 2023 · 1 comment
Open
5 tasks

Improve speed and API of local thickness #896

jgostick opened this issue Oct 13, 2023 · 1 comment
Labels
api PR tag for breaking changes

Comments

@jgostick
Copy link
Member

It's time to renovate the local thickness and porosimetry functions. The changes I have in mind are backwards incompatible so this is probably more like "add to the wish list for V3".

  • I could be imagining this, but I feel like at some point the edt function changed from defaulting to all cores, to just 1 core. Imagination or not, we should set parallel=porespy.setting.ncores.
  • We could alternatively not use edt and instead just use the _insert_spheres_at_points_parallel function, which I recently added to the tools module`. We'll have to do a good speed comparison. I guess we'll probably end up offering a 'mode' argument.
  • Speaking of mode we should also offer the skeleton based approach which is how it's done in imagej. Take the skeleton and just insert spheres at each point the skeleton, taking the radius from the underlying distance transform values. Or this could be its own function, like local_thickness_sk.
  • On the above note, maybe we could offer a variety of functions, local_thickness_dt which uses distance transforms, and local_thickness_conv, local_thickness_morph, and local_thickness_sph. And then local_thickness is a general function which dispatches to the correct one depending on mode.
  • There is also the question of the memory footprint as noted in Lower memory footprint for porosimetry and local_thickness #840
@jgostick
Copy link
Member Author

jgostick commented Oct 13, 2023

Additional thoughts:

  • porosimetry should be moved to the simulations module, since that's what it is.
  • So the logic that is currently in porosimetry should be moved into the local_thickness function.
  • porosimetry can call the local_thickness function then trim the blobs that invaded too early to apply the access limitations function. The porosimetry function could also accept mode which would get passed on to local_thickness.
  • The sizes should be integers between 1 and dt.max(), instead of having float values. This would make it quite a bit easier to generate histograms.

@jgostick jgostick added the api PR tag for breaking changes label Oct 19, 2023
@jgostick jgostick changed the title Improve speed of local thickness Improve speed and api of local thickness Oct 23, 2023
@ma-sadeghi ma-sadeghi changed the title Improve speed and api of local thickness Improve speed and API of local thickness Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api PR tag for breaking changes
Projects
None yet
Development

No branches or pull requests

1 participant