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

fit: number of overhanging blocks can be more than 1 #28

Open
pattonw opened this issue Dec 10, 2020 · 1 comment
Open

fit: number of overhanging blocks can be more than 1 #28

pattonw opened this issue Dec 10, 2020 · 1 comment

Comments

@pattonw
Copy link
Collaborator

pattonw commented Dec 10, 2020

Since "valid" fit would be missing at most 1 write roi block whose read roi might go out of the total roi, I would expect that an "overhang" fit would add at most 1 block on each axis. This is not the case though since a valid fit checks:
total_roi.contains(b.read_roi), and the "overhang" checks total_roi.contains(b.write_roi.get_begin()).

I've added an example to the documentation:

        "valid": Skip blocks that would lie outside of ``total_roi``. This
        is the default::

            |---------------------------|     total ROI

            |rrrr|wwwwww|rrrr|                block 1
                   |rrrr|wwwwww|rrrr|         block 2
                                            no further block

        "overhang": Add all blocks that overlap with ``total_roi``, even if
        they leave it. Client code has to take care of save access beyond
        ``total_roi`` in this case.::

            |---------------------------|     total ROI

            |rrrr|wwwwww|rrrr|                block 1
                   |rrrr|wwwwww|rrrr|         block 2
                          |rrrr|wwwwww|rrrr|  block 3 (overhanging)

            |---------------------------|     total ROI

            |rrrrrr|www|rrrrrr|                      block 1
                |rrrrrr|www|rrrrrr|                  block 2
                    |rrrrrr|www|rrrrrr|              block 3
                        |rrrrrr|www|rrrrrr|          block 4 (overhanging)
                            |rrrrrr|www|rrrrrr|      block 5 (overhanging)
                                |rrrrrr|www|rrrrrr|  block 6 (overhanging)

        "shrink": Like "overhang", but shrink the boundary blocks' read and
        write ROIs such that they are guaranteed to lie within
        ``total_roi``. The shrinking will preserve the context, i.e., the
        difference between the read ROI and write ROI stays the same.::

            |---------------------------|     total ROI

            |rrrr|wwwwww|rrrr|                block 1
                   |rrrr|wwwwww|rrrr|         block 2
                          |rrrr|www|rrrr|     block 3 (shrunk)

Documentation accurately describes the current functionality.

Is this meant to be or should we change the "overhang" check to something like total_write_roi.contains(b.write_roi.get_begin())

@cmalinmayor
Copy link
Collaborator

Will and I talked about this and think the best solution would be to calculate "valid", "overhang" or "shrink" on the write ROI only. The user can pass in either a total_write_roi or a total_read_roi, this can be internally converted to an internal total_write_roi, and then "valid" skips blocks whose write_roi lies outside of the total_write_roi. "overhang" adds all blocks whose write_roi overlaps with the total_write_roi. "shrink" shrinks the write_roi of the final block to fit within the total_write_roi. In every case, the read context stays the same.

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

No branches or pull requests

2 participants