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

[UIMA-6296] SelectFS fsIterator may yield different result set than asList #85

Conversation

reckart
Copy link
Member

@reckart reckart commented Nov 10, 2020

  • Add test of consitency of forward-iteration through select results with predicates

Selection types

FSIterator operations

  • forward iteration
  • backward iteration
  • moveTo
  • moveToFirst
  • moveToLast

Selection variations


Potential issues:

  • The precedence of limited+backwards+shifted for FOLLOWING / PRECEDING is different from COVERED_BY, COVERING, COLOCATED:
    • FOLLOWING / PRECEDING: backwards(limit(shifted(expected)))
    • COVERED_BY, COVERING, COLOCATED: limit(shifted(backwards(expected)))
  • shift + preceding: when using shift with preceding, the shift causes annotations close to the anchor to be skipped first. This might be surprising so it must be documented. E.g. consider you have [10, 20] [20, 30] and select().preceding([30, 40]), the result would be [10, 20] [20, 30] (in this order). So shift normally skips the result list from the beginning, so one might expect the result of select().preceding([30, 40]).shifted(1) to be [20, 30]. However, actually shift skips away from the reference point, so the result of select().preceding([30, 40]).shifted(1) is actually [10, 20]. I guess that if we take the view that shifted is good for selecting relative to the reference annotation instead of relative to the result list, that behavior is ok.
  • startAt is effectively mutually exclusive with other selectors like following, preceding, coveredBy, etc. It sets the startingFs field which completely ignored if boundsUse != none. If used with preceding or following, it effectively replaces the reference annotation from which the following/preceding selection is originating. In particular, it does not allow seeking to a particular annotation in the result of the following/preceding selection!

…sList

- Add test of consitency of forward-iteration through select results with predicates
@reckart reckart added the 🦟 Bug Something isn't working label Nov 10, 2020
@reckart reckart added this to the 3.2.0 milestone Nov 10, 2020
@reckart reckart self-assigned this Nov 10, 2020
…ield-different-result-set-than-asList

* master:
  [NO JIRA] Remove Eclipse Maven Repo
…sList

- Add test of consitency of back-to-front-iteration through select results with predicates
- Add warning when trying to use following/preceding with a negative shift. This worked but it heavily clashes with the ability to make iteration consistent with the asList behavior. Also, a better alternative to following/preceding with negative shift exists, namely using startAt.
…ield-different-result-set-than-asList

* master:
  [UIMA-6254] Move API report post-analysis script into the build resources
  [UIMA-6254] Move API report post-analysis script into the build resources
  [UIMA-6254] Move API report post-analysis script into the build resources
  [UIMA-6254] Move API report post-analysis script into the build resources
  [UIMA-6276] Potential memory leak in FSClassRegistry
…sList

- FsIterator_limited now limits by move operations and not by get operations
- Better FsIterator_limited moveTo operation which aims to respect the limit
- Fixed another edge case when doing a select in combination with FSIterators
- Improved randomized test suite in SelectFsAssert
- Move unit tests in SelectFsTest
- Switch from assertCountLimit to the more general assertCount in AnnotationIteratorTest
…sList

- Fixed various issues encountered when using the moveTo operation in combination with other operations (moveNext/movePrev) on FSIterators obtained via SelectFS
- Added various unit tests
- Improved randomized test harness
- Added debugging code to FsIterator_subtypes_ordered turned off via a static final boolean constant
…sList

- Fixed issues involving the limit operation
- Refactord tests and added more tests
-
…sList

- Fixed issues involving non-ambiguous selection
@reckart reckart changed the base branch from master to main December 3, 2020 16:23
…sList

- Adjust the annotation predicate matrix once more to avoid awkward situations in unit tests that before required an explanatory comment
- Also, it is now possible to reduce all predicates to coveredBy and there are new tests that include these reductions and ensure that the actual optimized production implementation of the predicates is aligned with the "axiomatic" predicate definition
- If we can define all predicates in terms of coveredBy, that means that we can implement basically all of the SelectFS operations by internally mapping them to a "coveredBy" selection. Doing this would likely reduce the complexity of the SelectFS code quite a bit. It will need to be seen if this reduction of code can be done without any significant loss of performance.
…ld-different-result-set-than-asList

* main:
  [UIMA-6295] CAS transportable Java object not serialised or deserialised with compressed binary
  [UIMA-6295] CAS transportable Java object not serialised or deserialised with compressed binary
  [UIMA-6295] CAS transportable Java object not serialised or deserialised with compressed binary
  [UIMA-6295] CAS transportable Java object not serialised or deserialised with compressed binary
  [UIMA-6295] CAS transportable Java object not serialised or deserialised with compressed binary
  [UIMA-6295] CAS transportable Java object not serialised or deserialised with compressed binary
  [UIMA-6301] Rename "master" branches to "main"
  [UIMA-6295] CAS transportable Java object not serialised or deserialised with compressed binary
…sList

- Added test for basic backwards selection
- Slightly refactored SelectFsAssert to better organize the sub-asserts within the randomized tests
@reckart
Copy link
Member Author

reckart commented Jan 14, 2021

Jenkins, can you test this please?

…sList

- Added additional tests for backwards selection in conjunction with non-overlapping and with limit
…sList

- Added test combining non-overlapping, backwards and limit.
…sList

- Added test combining non-overlapping and limit.
- More refactoring
…sList

- Change behavior of shifted by capping it to 0 if it tries moving beyond the selection boundary
- Added tests checking the behavior of shifted for default ambigous selects
…sList

- Change behavior of shifted by capping it to 0 if it tries moving beyond the selection boundary
- Removed unused fields
…sList

- Added test for ambigous + backwards + limited + shifted
…sList

- Added test for unambigous + backwards + limited + shifted
…ok-to-Asciidoc' into bugfix/UIMA-6296-SelectFS-fsIterator-may-yield-different-result-set-than-asList

* feature/UIMA-6305-Convert-UIMAv3-Users-Guide-from-DocBook-to-Asciidoc:
  [UIMA-6305] Convert UIMAv3 User's Guide from DocBook to Asciidoc
  [UIMA-6305] Convert UIMAv3 User's Guide from DocBook to Asciidoc
  [No Jira] Adjust branch names in GitHub metadata

% Conflicts:
%	uima-doc-v3-users-guide/src/docs/asciidoc/images/version_3_users_guide/annotation_predicates/annotation-relations-definition.png
%	uima-doc-v3-users-guide/src/docs/asciidoc/images/version_3_users_guide/annotation_predicates/annotation-relations-table.png
%	uima-doc-v3-users-guide/src/docs/asciidoc/images/version_3_users_guide/annotation_predicates/annotation-relations.png
%	uima-docbook-v3-users-guide/src/docbook/uv3.annotation_predicates.xml
%	uima-docbook-v3-users-guide/src/docbook/uv3.select.xml
@reckart reckart merged commit bb9e3b4 into main Jan 20, 2021
@reckart reckart deleted the bugfix/UIMA-6296-SelectFS-fsIterator-may-yield-different-result-set-than-asList branch January 20, 2021 09:32
reckart added a commit that referenced this pull request Sep 23, 2024
…iption-does-not-discover-type-priorities

[UIMA-6204] createReaderDescription does not discover type priorities
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🦟 Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants