PatBall1 / detectree2

Python package for automatic tree crown delineation based on the Detectron2 implementation of Mask R-CNN
https://patball1.github.io/detectree2/
MIT License
149 stars 35 forks source link

Predicting, reprojecting and F1 score #6

Closed James-Hirst-1998 closed 2 years ago

James-Hirst-1998 commented 2 years ago

Code Edits:

  1. Tiling.py - Takes the names of the files and tests whether the box overlaps with any of the test files. I had to swap the split of the test and train data so the test data was all done first and this meant that to select the test files I just picked the first few numbers in the randomised list.

  2. Training.py - Can now delete registered data as slow to do in colab as have to restart all the code. Also, new code to make predictions on the test dataset and save the resulting automatic crowns in a new folder.

  3. Reproject.py - Changes json file produced from the prediction and changes it to geojson format. It saves two files currently, one has EPSG code 26917 which means the file overlays the png file in qgis, the other requires the correct EPSG code so that the crowns overlay with the test tile and hence allows for them to be compared.

  4. CalculatingF1.py - All the functions needed to calculate the F1 scores for a site from all the test files. Need to add in a function to compare the height but currently works out areas of crown polygons and the intersection area and then outputs a list of all the intersections. This is used to get the number of true/ false positives and hence the F1 score is calculated.

ma595 commented 2 years ago

Hi James, thanks for the PR. It looks good to me.

Just a few notes about PRs in general. First of all, commit descriptions don't necessarily need underscores - simple comments will do to document changes made.

I notice that you've worked on the local master of your fork, while this is fine, it's customary to branch off master on a fork and create a PR from there. There's nothing wrong with the what you've done which is:

Fork/branch off detectree/master
Do some commits
Then merge/PR back

That's a consecutive set of commits that git will probably insert cleanly. But consider the following case:

If you fork/branch
Commit some stuff
Pull/merge from master
Commit some more stuff
Then merge/PR back

This can make the git history look quite messy (and other issues can arise) especially if other people do the same simultaneously.

Good practice is to:

Create a branch from your fork
Commit some stuff
Then rebase on origin/master when ready to submit a PR

Rebasing on master is well described in this stackoverflow post as well as the git-book.
https://stackoverflow.com/questions/7929369/how-to-rebase-local-branch-onto-remote-master https://git-scm.com/book/en/v2/Git-Branching-Rebasing

The complexity of the rebase largely depends on the number of people contributing to your branch. We'll assume just one person - and a longwinded way of doing it is:

git checkout master
git pull upstream master  # I assume upstream points to patball1/detectree2 repo in your case. 
git checkout newfix
git rebase master # fix issues
git push --force-with-lease yourorigin name_of_branch # force is necessary here if modifying an existing PR.

This keeps master inline with PatBall1/detectree2 master, and after the rebase the commits will be added on top of HEAD.

ma595 commented 2 years ago

To resolve the conflict you can either rebase on master:

git fetch upstream master
git rebase upstream/master (and fix all issues that arise)

This pulls all changes made by James and me, and rebases your changes on top of it. Then do: git push --force-with-lease (rebasing keeps your git history clean, but you can merge as well if preferred)

git pull upstream master
git push origin master

OR

branch your changes
reset head on master
and resubmit the PR having rebased on origin/master as discussed above. 

Sorry if this is all a little unclear. Let me know if you have any questions.

ma595 commented 2 years ago

In fact, having looked at the conflicts, they look quite simple to resolve using the editor.