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

Downscale factor has no effect #46

Closed
Breakthrough opened this issue Feb 3, 2021 · 4 comments
Closed

Downscale factor has no effect #46

Breakthrough opened this issue Feb 3, 2021 · 4 comments

Comments

@Breakthrough
Copy link
Owner

Breakthrough commented Feb 3, 2021

Currently, downscale factor has no effect if set (i.e. is wholly unused). If set, also needs to re-calculate the kernel size appropriately, and ensure ROI selection is not affected (but scales properly).

Discovered while refactoring ScanContext as part of #33.

@Breakthrough Breakthrough added this to the v1.2 milestone Feb 3, 2021
Breakthrough added a commit that referenced this issue Feb 7, 2021
Other minor fixes (e.g. order arguments the same in the
setters as they are defined in ScanContext).
@Breakthrough
Copy link
Owner Author

Resolved in v1.2 branch, to be included in next release of DVR-Scan.

@ocram
Copy link

ocram commented Dec 10, 2021

If set, also needs to re-calculate the kernel size appropriately, and ensure ROI selection is not affected (but scales properly).

Has this (specifically, the first part) actually been implemented?

From the commit above, it seems the kernel size is only re-calculated automatically for implicit (automatically used) values, i.e. when you provide no explicit kernel size yourself. Maybe that’s all that is desired here. But what this means is that whenever you provide a kernel size and then change the downscale factor, it stops working as expected, and you have to adjust the kernel size manually again (to match the changed downscale factor).

As an example, if I start with --downscale-factor 1 --kernel-size 15 and then switch to …

  • --downscale-factor 2, I need to manually re-adjust to --kernel-size 7
  • --downscale-factor 4, I need to manually re-adjust to --kernel-size 3
  • --downscale-factor 8, I need to manually re-adjust to --kernel-size 3 (minimum reached?)

… for roughly equal results. Is this intended?

@Breakthrough
Copy link
Owner Author

Ah very good catch - this is not intended indeed. Will re-open this until it's fixed, thank you for the update!

@Breakthrough Breakthrough reopened this Dec 11, 2021
@Breakthrough Breakthrough modified the milestones: v1.2, v1.4 Dec 13, 2021
Breakthrough added a commit that referenced this issue Jan 25, 2022
Previously was incorrect due to downscale factor not being compensated
for kernel size was also set and not automatically calculated.

Fixes #46.
@Breakthrough
Copy link
Owner Author

Fixed in v1.4 branch, will be included in next release of DVR-Scan. Thanks @ocram!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants