Refactor Radial Distribution Function (RDF)#43
Refactor Radial Distribution Function (RDF)#43leifdenby wants to merge 4 commits intocloudsci:masterfrom
Conversation
leifdenby
commented
Jan 19, 2022
- complete refactoring of rdf function
- implement test
|
@martinjanssens I'm a bit confused about what the RDF function should be returning. I guess what it is calculating internally is the probability distribution function as a density (i.e. probability of funding a mask at a given distance). Currently that is as a function of radial distance Is there any reason why the distance couldn't be given in pixels? I realise that fractional indices don't make a lot of sense since the pixel indices are integers, but that shouldn't matter for the distribution, no? What I am suggesting is that the grid resolution becomes an optional argument and if provided then it is assumed to be in meters and all the returned values scaled by this number. Does that make sense? |
|
Yes, I think your analysis is completely right and your proposal makes sense. As you say |
Use rdfpy package as reference for testing RDF code on test case where points are sampled using modified Poisson Disk Sampling, where we can control the point spacing.
|
Thanks for your feedback @martinjanssens! I've spent this morning working on this and here are a few thoughts:
All of this is making me think that maybe we should push RDF to |
I'm so slow I hadn't even realised we'd already agreed to do this :) sorry |
|
Great, thanks for looking at this in so much detail! I've had a look at the code, and I think you're right to be confused regarding point 1. I think this was my thought:
Do you agree? :) |
|
Using an existing package is a great idea, as is using your Poisson disc sampling as a reference case! I can take a look at where the differences with our implementation come from, but they may very well be discretisation artefacts related to binning the RDF, which I think you can theoretically correct for (and we don't). |
|
And finally, yes let's pick this up for the next version, I agree we'll probably be playing around with this for a while still! |
