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

[#838] add random edge sampling #839

Merged
merged 9 commits into from
Jun 29, 2018
Merged

[#838] add random edge sampling #839

merged 9 commits into from
Jun 29, 2018

Conversation

rostam
Copy link
Contributor

@rostam rostam commented Jun 27, 2018

fixes #838

Copy link
Contributor

@merando merando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether this has to be an UnaryGraphOperator. IMO this should be a FilterFunction generalized for EPGMElements which then can be used as part of subGraph operations


/**
* seed for the random number generator
* if seed is null, the random generator is created without seed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the seed can't be null since you are using a primitive type

* Creates new RandomEdgeSampling instance.
*
* @param sampleSize relative sample size
* @param randomSeed random seed value (can be {@code null})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, can't be null. to do so you would need 'Long' instead of 'long'. But the later code snippets indicate that your are refering to '0' instead of 'null' so a fix of the java doc should be sufficient

* and their associated source- and target-vertices. No unconnected vertices will retain in the
* sampled graph.
*/
public class RandomEdgeSampling implements UnaryGraphToGraphOperator {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't this be implemented as a FilterFunction which then is applied in a edgeInducedSubgraph call?
Isn't this generalizable for Vertices also?

Copy link
Contributor Author

@rostam rostam Jun 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There will be more sampling methods in which either vertices or edges or both are filtered.
In this special case, it may be thought of a simple filter function however it is not
the case in all samplings. In some sampling, it should be an unary graph to graph operator. So to have a bundle of similar algorithms, we just call all of them samplings.
For example, just think of PageRankSampling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also corrected the comments regarding the random seed.
So it can be zero not null.

@galpha galpha mentioned this pull request Jun 28, 2018
10 tasks
@galpha
Copy link
Contributor

galpha commented Jun 28, 2018

@merando pls see #835

Copy link
Contributor

@galpha galpha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@galpha galpha merged commit c8e99d5 into dbs-leipzig:master Jun 29, 2018
0x002A pushed a commit to ChrizZz110/gradoop that referenced this pull request Feb 19, 2019
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

Successfully merging this pull request may close these issues.

3 participants