lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (LUCENE-3501) random sampler is not random (and so facet SamplingWrapperTest occasionally fails)
Date Sun, 09 Oct 2011 16:13:29 GMT

    [ https://issues.apache.org/jira/browse/LUCENE-3501?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13123717#comment-13123717
] 

Shai Erera commented on LUCENE-3501:
------------------------------------

I briefly went through the patch:

* In the test, I prefer to still catch Exception, or if you want to be on the safe side, Throwable.
And have assertSameResult throw RuntimeException. Calling fail() from there, forcing you to
handle Errors, seem too low-level to me ...

* In AdaptiveFacetsAccumulator you have this line: //private Sampler sampler = new RepeatableSampler();.
Is it a leftover?

* Maybe add a line or two to RandomSample.createSample() (internal comments), such as:
** "Skip over 'step' documents" before the for-loop
** "Add all leftover documents to the sampled set" before last 'while'.
(Please also confirm my understanding) :)

* CHANGES - in this case you only need a CHANGES entry for 3x (since the change is applied
to there as well), and it is under contrib.

Otherwise, it looks very good !
                
> random sampler is not random (and so facet SamplingWrapperTest occasionally fails)
> ----------------------------------------------------------------------------------
>
>                 Key: LUCENE-3501
>                 URL: https://issues.apache.org/jira/browse/LUCENE-3501
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: modules/facet
>            Reporter: Doron Cohen
>            Assignee: Doron Cohen
>            Priority: Minor
>         Attachments: LUCENE-3501.patch
>
>
> RandomSample is not random at all:
> It does not even import java.util.Random, and its behavior is deterministic.
> in addition, the test testCountUsingSamping() never retries as it was supposed to (for
taking care of the hoped-for randomness).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message