Adding disjunctive query processing, and heaps/maxScore considerations in UnionIterator.php.
Please find the explanation for the same at: http://www.cs.sjsu.edu/faculty/pollett/masters/Semesters/Spring23/gargi/index.php?DisjunctiveQueries.html
gargisheguri2023-11-12 07:30
Modifications: to fix warnings observed on testing in findcan.ca
---
Warning: Undefined property: seekquarry\yioop\library\index_bundle_iterators\NegationIterator::$word_key in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/UnionIterator.php on line 348
Deprecated: str_starts_with(): Passing null to parameter #1 ($haystack) of type string is deprecated in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/UnionIterator.php on line 352
and
Deprecated: Implicit conversion from float 0.0019202864496037364 to int loses precision in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/WordIterator.php on line 718
Deprecated: Implicit conversion from float 0.0019202864496037364 to int loses precision in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/WordIterator.php on line 744
---
Fixes:
- UnionIterator::findDocsWithWord()- use $word_key property only if present on the iterator
- WordIterator::getDocKeyPositionsScoringInfo()- use intval of $num_description_scores for array_slice() operations
Can you add to the issue what's changed between your patches? The issue tracker tracker is supposed to track in English historical changes to the software.
Best,
Chris
gargisheguri2023-11-12 08:20
Thanks so much Professor, of course, let me update that
Best,
Gargi
gargisheguri2023-11-12 10:06
Added flag to toggle between conjunctive and disjunctive queries: Config.php::USE_CONJUNCTIVE_QUERY
gargisheguri2023-11-12 10:29
Added flag to toggle between conjunctive and disjunctive queries: Config.php::USE_CONJUNCTIVE_QUERY
Fixes to disjunctive query creation logic: for result descriptions and disallowed words
- using regex to avoid disallowed search terms in parseWordStructDisjunctiveQuery()
- fixed format words overwrite on each parseWordStructConjunctiveQuery()
1. Modifications to heap/maxScore logic in UnionIterator
2. Extraction of lookup for most recent version of page into IndexManager (with URLs cached)
3. ArcTool fix for bloom filter addition in doc_map
Thanks for your patch! For the most part it looks good. Here are some comments:
(1) Don't make changes to IndexBundleIterator so that it depends on a subclass.
Figure out how to get the effect of
@@ -193,8 +193,10 @@ abstract class IndexBundleIterator implements CrawlConstants
*/
public function currentDocsWithWord()
{
+ $is_union = $this instanceof UnionIterator;
if ($this->current_block_fresh == true) {
- return $this->pages;
+ return $is_union && is_array($this->pages) ?
+ count($this->pages) : $this->pages;
by only changing UnionIterator. You are also not allowed to change the return type of currentDocsWithWord()
to make it a mixed type like you did.
(2) The WordIterator code below is probably slow:
@@ -798,6 +790,9 @@ class WordIterator extends IndexBundleIterator
*/
public function getMaxScore()
{
+ if (L\PhraseParser::checkMetaTerm($this->word_key)) {
+ return 0.01;
+ }
Call L\PhraseParser::checkMetaTerm in the constructor, set a flag with its value, then use that flag in getMaxScore
Best,
Chris
administrator2023-11-29 23:08
Hi Gargi,
Looking at your code some more, another issue I see is that the pages output from your UnionIterator should have CrawlConstants::RELEVANCE and CrawlConstants::DOC_RANK and CrawlConstants::SCORE to the correct values computed by the union or later sorting won't work correctly. Also, you should try to minimize having arrays with fields other than CrawlConstants. If you mispell or change CrawlConstants::SCORE you are likely to get a quickly detected error, but a field "SCORE" might only give a warning or no problem whatsover even though there is a bug.
Best,
Chris
gargisheguri2023-11-29 23:14
Thanks so much for your suggestions Professor! I'll fix those ASAP.
Could you please help me understand this: 'the pages output from your UnionIterator should have CrawlConstants::RELEVANCE and CrawlConstants::DOC_RANK and CrawlConstants::SCORE to the correct values computed by the union or later sorting won't work correctly.'?
I will update the constant values to be declared in CrawlConstants instead of typing out the strings in UnionIterator, but just wanted to verify what the former part of the comment meant, thank you!
Best,
Gargi
administrator2023-11-30 06:36
UnionIterator through IndexBundleIterator implements CrawlConstants, so you can use self::DOC_RANK, self::SCORE, etc. Basically, the idea is that anything that anything that implements CrawlConstants will use the same field names for things. Any call to currentDocsWithWord in IndexBundleIterator or its subclasses should have self::DOC_RANK, self::RELEVANCE, self::SCORE correctly computed and set for each posting produced by those methods. You shouldn't be changing return types of this method or findDocsWithWord. Ideally, I should have written a class. Then each of the pages returned by these methods would be of that class type. Using CrawlConstants, though, lets me have some of the checking of having made a class, but let's me have some of the flexibility of easily tacking on new fields of associative arrays.
On a slightly related note, your code in getPagesToGroup in GroupIterator shouldn't have to have a special case for UnionIterator. The logic should be the same for all iterators or you are likely to get some weird things breaking in the future. Put the logic you have in GroupIterator into the relevant methods in UnionIterator.
gargisheguri2023-11-30 09:51
Thanks very much for your help! Updated the patch with your suggestions, hope this is better.
The most recent patch seems to be better. I will make some tweaks to it before pushing it. Maybe later in the afternoon (eating lunch first), do a pull, and use that for your experiments.
Best,
Chris
gargisheguri2023-11-30 10:30
Sure Professor, thank you so much for everything, really appreciate it
Best,
Gargi
administrator2023-11-30 18:22
Hey Gargi,
I pushed your code. You can do a git diff to see some minor changes I made. I'll probably test it out for a couple days before closing this ticket. Thanks a lot for all your work.
Best,
Chris
gargisheguri2023-11-30 23:01
Thanks so much for everything, Professor! I'll update the report as well.
Adding disjunctive query processing, and heaps/maxScore considerations in UnionIterator.php. Please find the explanation for the same at: http://www.cs.sjsu.edu/faculty/pollett/masters/Semesters/Spring23/gargi/index.php?DisjunctiveQueries.html
Modifications: to fix warnings observed on testing in findcan.ca --- Warning: Undefined property: seekquarry\yioop\library\index_bundle_iterators\NegationIterator::$word_key in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/UnionIterator.php on line 348
Deprecated: str_starts_with(): Passing null to parameter #1 ($haystack) of type string is deprecated in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/UnionIterator.php on line 352
and
Deprecated: Implicit conversion from float 0.0019202864496037364 to int loses precision in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/WordIterator.php on line 718
Deprecated: Implicit conversion from float 0.0019202864496037364 to int loses precision in /Applications/MAMP/htdocs/git/yioop/src/library/index_bundle_iterators/WordIterator.php on line 744 ---
Fixes: - UnionIterator::findDocsWithWord()- use $word_key property only if present on the iterator - WordIterator::getDocKeyPositionsScoringInfo()- use intval of $num_description_scores for array_slice() operations
disjunctqueries.patch
Hey Gargi,
Can you add to the issue what's changed between your patches? The issue tracker tracker is supposed to track in English historical changes to the software.
Best, Chris
Thanks so much Professor, of course, let me update that Best, Gargi
Added flag to toggle between conjunctive and disjunctive queries: Config.php::USE_CONJUNCTIVE_QUERY
Added flag to toggle between conjunctive and disjunctive queries: Config.php::USE_CONJUNCTIVE_QUERY
flagdisjunctqueries.patch
Fixes to disjunctive query creation logic: for meta word detection
disjfixes.patch
Fixes to disjunctive query creation logic: for meta word detection: - using regex to detect meta words in parseWordStructDisjunctiveQuery()
disjmeta.patch
Fixes to disjunctive query creation logic: for news: - using regex to avoid multiple spaces between search terms in parseWordStructDisjunctiveQuery()
disjnews.patch
Fixes to disjunctive query creation logic: for result descriptions and disallowed words - using regex to avoid disallowed search terms in parseWordStructDisjunctiveQuery() - fixed format words overwrite on each parseWordStructConjunctiveQuery()
disjdescription.patch
Fixes to disjunctive query creation logic: trim for single query words with no:guess
disjtrim.patch
1. Modifications to heap/maxScore logic in UnionIterator 2. Extraction of lookup for most recent version of page into IndexManager (with URLs cached) 3. ArcTool fix for bloom filter addition in doc_map
changes.patch
Hey Gargi,
Thanks for your patch! For the most part it looks good. Here are some comments: (1) Don't make changes to IndexBundleIterator so that it depends on a subclass. Figure out how to get the effect of @@ -193,8 +193,10 @@ abstract class IndexBundleIterator implements CrawlConstants
*/ public function currentDocsWithWord() {+ $is_union = $this instanceof UnionIterator;
if ($this->current_block_fresh == true) {- return $this->pages; + return $is_union && is_array($this->pages) ? + count($this->pages) : $this->pages; by only changing UnionIterator. You are also not allowed to change the return type of currentDocsWithWord() to make it a mixed type like you did. (2) The WordIterator code below is probably slow: @@ -798,6 +790,9 @@ class WordIterator extends IndexBundleIterator
*/ public function getMaxScore() {+ if (L\PhraseParser::checkMetaTerm($this->word_key)) { + return 0.01; + } Call L\PhraseParser::checkMetaTerm in the constructor, set a flag with its value, then use that flag in getMaxScore
Best, Chris
Hi Gargi,
Looking at your code some more, another issue I see is that the pages output from your UnionIterator should have CrawlConstants::RELEVANCE and CrawlConstants::DOC_RANK and CrawlConstants::SCORE to the correct values computed by the union or later sorting won't work correctly. Also, you should try to minimize having arrays with fields other than CrawlConstants. If you mispell or change CrawlConstants::SCORE you are likely to get a quickly detected error, but a field "SCORE" might only give a warning or no problem whatsover even though there is a bug.
Best, Chris
Thanks so much for your suggestions Professor! I'll fix those ASAP. Could you please help me understand this: 'the pages output from your UnionIterator should have CrawlConstants::RELEVANCE and CrawlConstants::DOC_RANK and CrawlConstants::SCORE to the correct values computed by the union or later sorting won't work correctly.'?
I will update the constant values to be declared in CrawlConstants instead of typing out the strings in UnionIterator, but just wanted to verify what the former part of the comment meant, thank you!
Best, Gargi
UnionIterator through IndexBundleIterator implements CrawlConstants, so you can use self::DOC_RANK, self::SCORE, etc. Basically, the idea is that anything that anything that implements CrawlConstants will use the same field names for things. Any call to currentDocsWithWord in IndexBundleIterator or its subclasses should have self::DOC_RANK, self::RELEVANCE, self::SCORE correctly computed and set for each posting produced by those methods. You shouldn't be changing return types of this method or findDocsWithWord. Ideally, I should have written a class. Then each of the pages returned by these methods would be of that class type. Using CrawlConstants, though, lets me have some of the checking of having made a class, but let's me have some of the flexibility of easily tacking on new fields of associative arrays.
On a slightly related note, your code in getPagesToGroup in GroupIterator shouldn't have to have a special case for UnionIterator. The logic should be the same for all iterators or you are likely to get some weird things breaking in the future. Put the logic you have in GroupIterator into the relevant methods in UnionIterator.
Thanks very much for your help! Updated the patch with your suggestions, hope this is better.
Best, Gargi
changes-2.patch
Hey Gargi,
The most recent patch seems to be better. I will make some tweaks to it before pushing it. Maybe later in the afternoon (eating lunch first), do a pull, and use that for your experiments.
Best, Chris
Sure Professor, thank you so much for everything, really appreciate it
Best, Gargi
Hey Gargi,
I pushed your code. You can do a git diff to see some minor changes I made. I'll probably test it out for a couple days before closing this ticket. Thanks a lot for all your work.
Best, Chris
Thanks so much for everything, Professor! I'll update the report as well.
Best, Gargi
Attachments:
disjqueries.patch