search

linkagescape / linkage-mapper

ArcGIS tools to automate mapping and prioritization of wildlife habitat corridors
https://circuitscape.org/linkagemapper/
GNU General Public License v3.0
37 stars 12 forks source link

Log file improvements – fixes #101 and #102 #142

Closed venkanna37 closed 2 years ago

venkanna37 commented 3 years ago

@dkav and @johngallo please go through the code and output files that will send you through the mail. I can change and send the pull request again according to your feedback.

dkav commented 2 years ago

@venkanna37 I updated your commits. Let me know if you have any questions regarding my changes or if you think there should be further revisions.

Here is the new output: 2021_07_08_1213_Linkage Mapper.txt 2021_07_08_1356_Linkage Priority.txt 2021_07_08_1241_Climate Linkage Mapper.txt 2021_07_08_1225_Barrier mapper.txt 2021_07_08_1236_Circuitscape (Centrality).txt 2021_07_08_1239_Circuitscape (Pinchpoint).txt

johngallo commented 2 years ago

At the high level: looking good team! (I have not reviewed the code changes though, but trust that they are great.)

Idea for version 3.1 or later : add to the cores metadata: "Area of Smallest Core":

I say this because in rare occasions the smallest core is smaller than a resistance cell, and that crashes the model run, last I remember. For now, I propose we move on and get this release out!

On Thu, Jul 8, 2021 at 3:22 PM Darren Kavanagh @.***> wrote:

@venkanna37 https://github.com/venkanna37 I updated your commits. Let me know if you have any questions regarding my changes or if you think there should be further revisions.

Here is the new output: 2021_07_08_1213_Linkage Mapper.txt https://github.com/linkagescape/linkage-mapper/files/6787578/2021_07_08_1213_Linkage.Mapper.txt 2021_07_08_1356_Linkage Priority.txt https://github.com/linkagescape/linkage-mapper/files/6787584/2021_07_08_1356_Linkage.Priority.txt 2021_07_08_1241_Climate Linkage Mapper.txt https://github.com/linkagescape/linkage-mapper/files/6787583/2021_07_08_1241_Climate.Linkage.Mapper.txt 2021_07_08_1225_Barrier mapper.txt https://github.com/linkagescape/linkage-mapper/files/6787580/2021_07_08_1225_Barrier.mapper.txt 2021_07_08_1236_Circuitscape (Centrality).txt https://github.com/linkagescape/linkage-mapper/files/6787581/2021_07_08_1236_Circuitscape.Centrality.txt 2021_07_08_1239_Circuitscape (Pinchpoint).txt https://github.com/linkagescape/linkage-mapper/files/6787582/2021_07_08_1239_Circuitscape.Pinchpoint.txt

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/linkagescape/linkage-mapper/pull/142#issuecomment-876783016, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD7CJ25QMDWAMDVXHRNXW3LTWYQK7ANCNFSM47LLYAUQ .

dkav commented 2 years ago

@johngallo this change was not intended for the 3.0 release. Also, I think the small core issue should be part of a validation check or fix.

johngallo commented 2 years ago

The log files look great! Approving now.

Regarding our conversation: Dealing with the small cores as part of validation check would be great. Or even in the error message as a fallback. (Not sure what a validation fix would look like, but sounds more complicated.)

I figured this commit one of the commits that you mentioned that are candidates for the v3.0 release. Was hoping to discuss those candidates in our next meeting. Maybe an "issue" thread is better for speed.

Regardless, since it is us getting the v 3.0 out, and all that is on your plate right at the moment, I was figuring would be best to focus on that release rather than this validation code idea now. Do you have a targeted timeline for adding your changes to the .tbx and .docx in my latest pull request?

dkav commented 2 years ago

@johngallo you didn't give @venkanna37 time to respond before merging. Also I would prefer if you did a rebase and merge commit rather than a merge commit (it keeps a clearer history).

johngallo commented 2 years ago

@dkav sorry about that. That was the only option I saw for approving the review. I can press "revert" which starts a new pull request so @venkanna37 can compare the changes. I'll do that unless I hear otherwise before I need to leave (11 minutes). Regardless, I approve the review.

dkav commented 2 years ago

@johngallo I will take care of it.

venkanna37 commented 2 years ago

@dkav @johngallo I thought of responding after going to lab. I will check and reply soon.

dkav commented 2 years ago

@johngallo Info on rebase and merge pull requests.

venkanna37 commented 2 years ago

@dkav @johngallo I had a look at the code and log files, log files look great. As a next step, I have done some experiments on integrating Circuitscape-5 with Pinchpoint mapper and Centrality mapper. We can use CS5 as optional in those two tools, in the next week, I will send more detailed mail or post in google chat.