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

[feature][processing] Complete random raster algorithm collection #36065

Closed
wants to merge 20 commits into from

Conversation

root676
Copy link
Contributor

@root676 root676 commented Apr 28, 2020

Description

Some random stuff again...

This PR refactors and completes the recently added work on random number raster layer creation algorithms (see #35835.) With the new algorithms aimed towards distribution based random number creation, QGIS reaches to the same level of functionality as current ArcGIS random raster creation tools. In total, the PR adds the following algorithms (normal and uniform raster layer creation algs are refactored to be in line the new naming scheme):

  • Create random raster layer (binomial distribution)
  • Create random raster layer (exponential distribution)
  • Create random raster layer (gamma distribution)
  • Create random raster layer (negative binomial distribution)
  • Create random raster layer (normal distribution)
  • Create random raster layer (poisson distribution)
  • Create random raster layer (uniform distribution)

Tests have been added for the algorithms data type choices.
I'm looking forward to comments/questions/reviews/etc.!

…al distribution) and fix header

fix normal raster
@github-actions github-actions bot added this to the 3.14.0 milestone Apr 28, 2020
@nyalldawson
Copy link
Collaborator

@root676 this is GREAT! 😄

Looking at the algorithms, there's a lot of common code between them all (basically everything after the creation of the random distribution is identical). I think we could avoid a lot of the duplicate code by making a common base class for the algorithms. Are you comfortable doing this?

@root676
Copy link
Contributor Author

root676 commented Apr 30, 2020

Thanks, Yes I thought about this too... the first time I wanted to implement the algorithms I actually tried to do it with a base class (similar to the Fuzzify algs). But then I ran into some troubles because the random number distribution objects and available raster data types vary (also in type) among all algorithms. I saw no chance in using them as an argument in a pure virtual obtainRandomNumber(distribution) method). Do you have any hint on how to solve this with a design pattern (maybe strategy pattern: encapsulate the things that vary...)? I'll sure give it a try! 👍

@root676
Copy link
Contributor Author

root676 commented Apr 30, 2020

I think I have got a solution to the problem - I'll push the changes in a new PR and close this one. What do you think?

@root676 root676 closed this May 1, 2020
@root676 root676 deleted the processing_randomRaster branch May 1, 2020 17:57
nyalldawson pushed a commit that referenced this pull request May 3, 2020
This refactors and completes the recently added work on random number raster layer creation algorithms (see #35835) and reworks the single algorithm implementation proposed in #36065 to a base-algorithm solution which avoids duplicate code.

With the new algorithms aimed towards distribution based random number creation, QGIS reaches to the same level of functionality as current ArcGIS random raster creation tools. In total, the PR adds the following algorithms (normal and uniform raster layer creation algs are refactored to be in line the new naming scheme):

Create random raster layer (binomial distribution)
Create random raster layer (exponential distribution)
Create random raster layer (gamma distribution)
Create random raster layer (negative binomial distribution)
Create random raster layer (normal distribution)
Create random raster layer (poisson distribution)
Create random raster layer (uniform distribution)
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.

2 participants