NanoComp / imageruler

measure minimum solid/void lengthscales in binary image
https://nanocomp.github.io/imageruler/
MIT License
13 stars 6 forks source link

Violation functions and duality tests #5

Closed mawc2019 closed 1 year ago

mawc2019 commented 1 year ago

This PR changes the following items, none of which affect the values of minimum length scales and the description in the paper.

ianwilliamson commented 1 year ago

Do these new length_violation_solid() and length_violation_void() functions compute intermediate quantities used inside of minimum_length_solid() and minimum_length_void(), respectively? Is it possible to refactor the latter to utilize the former?

mawc2019 commented 1 year ago

Do these new length_violation_solid() and length_violation_void() functions compute intermediate quantities used inside of minimum_length_solid() and minimum_length_void(), respectively?

Let us take the solid case as an example. The functions length_violation_solid() and minimum_length_solid() have some overlap, but the former is not entirely in the latter.

Is it possible to refactor the latter to utilize the former?

I also thought about this. Refactoring can make the logic clearer, but in this case it may not seem worth doing. In minimum_length_solid(), violation is calculated multiple times because of binary search. Before that, an initialization step is performed by _ruler_initialize(). On the other hand, in length_violation_solid(), after the initialization _ruler_initialize(), violation is calculated only once for a given input length scale. Therefore, if we put length_violation_solid() inside minimum_length_solid(), during binary search, _ruler_initialize() would run multiple times, which is unnecessary and ineffective. Another approach is to isolate _ruler_initialize() from length_violation_solid() and write a simpler function for solid violation. However, such a function without initialization is not suitable for a user function. To prepare the user function, we still need another function that invokes both that simpler function and _ruler_initialize(). This approach results in more functions make makes the code longer. In the latest commit I made such revision, but we may not adopt it.