GispoCoding / eis_toolkit

Python library for mineral prospectivity mapping
https://eis-he.eu/
European Union Public License 1.2
26 stars 8 forks source link

proximity tool to convert the 0's and 1's in the polygon #432

Closed okolekar closed 1 month ago

okolekar commented 1 month ago

As discussed during the meetings, where it was requested to add a new tool that can convert the 0's and 1's in the polygon has been created. I have added the new proximity tool to invert the 0's and 1's in the polygon we get from the distance computation. I have just used only linear interpolation as of now.

image

nmaarnio commented 1 month ago

Hey, a quick review: The tool should not take in an already computed distance array, but instead perform the distance computation within this proximity tool. It is already possible to first run distance computation and then apply whatever transformation/scaling to the output, but now we want convenience (=user runs just one tool to produce the proximity raster). Also, maximum distance parameter should not be optional here, since the output raster does not really make sense if the values scale up to the boundaries (unless the user specifically sets the max distance so large that it reaches the edges).

nmaarnio commented 1 month ago

Also see issue #430 which was made for this feature

okolekar commented 1 month ago

Hey, a quick review: The tool should not take in an already computed distance array, but instead perform the distance computation within this proximity tool. It is already possible to first run distance computation and then apply whatever transformation/scaling to the output, but now we want convenience (=user runs just one tool to produce the proximity raster). Also, maximum distance parameter should not be optional here, since the output raster does not really make sense if the values scale up to the boundaries (unless the user specifically sets the max distance so large that it reaches the edges).

Okay, I get it now.

okolekar commented 1 month ago

Also see issue #430 which was made for this feature

So should I make a commit there or should it be here (I am referring to the pull request here should it be done to #430 or here)?

nmaarnio commented 1 month ago

No need to rename branch or anything, the issue you based this feature on was a little misleading (should have been closed already probably) and can be closed when this PR is merged. Additionally #430 will be closed when this is merged.

okolekar commented 1 month ago

The tool now takes in a GeoDataFrame, raster profile and maximum distance as inputs and performs the inversion by calling distance computation function inside the tool itself. Logarithmic scaling is also introduced as result of previous.

image

The logarithmic scaling results are shown below

image

nmaarnio commented 1 month ago

Any updates with this tool @okolekar? Since the last progress of EIS project is nearing and is less than a month away, it would be good if this tool was implemented and merged soon so that people have the opportunity to test it.

okolekar commented 1 month ago

Any updates with this tool @okolekar? Since the last progress of EIS project is nearing and is less than a month away, it would be good if this tool was implemented and merged soon so that people have the opportunity to test it.

I am very sorry for the delay, as we face a small challenge with the log transformation. However, I am done with the linear tool. The log transformation is a bit more tricky. As the results are not the same as what we expected. However, if the tool has to be tested I can upload the linear part of the proximity tool after removing the log transformation. Shall I proceed to upload with the linear part?

nmaarnio commented 1 month ago

No worries! Okay, I think in this case we should add the log transform in a later PR and proceed with just the linear one for now. Thanks

okolekar commented 1 month ago

Hello @nmaarnio, I have pushed the changes with the recommended changes. Sorry for the delay as I was working on log transformation. I did not make changes to the cli.py (command line interface) seems to take a bit more time. However, due to the urgency of the project I have skipped it in this commit. I hope that is okay. I will now work with the log transformations. However, I am available to make changes if needed.

nmaarnio commented 1 month ago

Thanks, I will do a review tomorrow and implement the CLI function 👍