findFeaturesSegment.py script fixes #5565

chkim-usgs · 2024-07-29T18:51:34Z

Description

The process to retrieve the from_segments in this line

from_segments = reduce(lambda d1,d2: {k: merge(d1, d2, k) for k in set(d1)|set(d2)}, output)

from a FROMLIST that contains only one cube outputs this format:

{ 'seg1': PVLGroup([...]), 'seg2': PVLGroup([...])}

so accessing each segment as a list caused a TypeError.

Added an if/else statement to account for img_list = 1 so from_segments ultimately outputs this:

{ 'seg1': [PVLGroup([...])], 'seg2': [PVLGroup([...])]}

To reduce redundant overlapping image pairs, I implemented an iteration of @Kelvinrr's generic code with the change in the way the segments are accessed.

Related Issue

Addresses #5489 & #5540

How Has This Been Validated?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Documentation change (update to the documentation; no code change)
Breaking change (fix or feature that would cause existing functionality to change)
Infrastructure change (changes to things like CI or the build system that do not impact users)

Checklist:

I have read and agree to abide by the Code of Conduct
I have read the CONTRIBUTING document.
My change requires a change to the documentation and I have updated the documentation accordingly.
I have added tests to cover my changes.
I have added myself to the .zenodo.json document.
I have added my user impacting change to the CHANGELOG.md document.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

Kelvinrr · 2024-07-30T14:34:52Z

isis/python_bindings/apps/findFeaturesSegment/findFeaturesSegment.py

@@ -134,7 +134,7 @@ def segment(img_path : Path, nlines : int = MAX_LEN):

 def generate_cnet(params, images):

-    match_segment_n = images["match"]["Segment"]
+    match_segment_n = images["match"][0]["Segment"]


Is this needed every time? Or only sometimes? I find it odd that it seemed to be working in some cases. Sometimes you have to check both cases.

Hm very true, I'll have to add some sort of check then. Good catch.

Kelvinrr · 2024-07-30T14:36:49Z

isis/python_bindings/apps/findFeaturesSegment/findFeaturesSegment.py

@@ -197,7 +197,7 @@ def generate_cnet(params, images):
            log.debug(f"overlap stats: {ret.stdout}")

            # first, throw it out if there is no overlap whatsoever 
-            is_pair_overlapping = not any([k[1].get("NoOverlap", "") == new_params["MATCH"] for k in stats])
+            is_pair_overlapping = any([k[1].get("NoOverlap", "") == new_params["MATCH"] for k in stats])


bruh that's a pretty bad bug you caught there

Kelvinrr · 2024-07-30T14:46:27Z

isis/python_bindings/apps/findFeaturesSegment/findFeaturesSegment.py

+    x = match_segments.values()
+    y = from_segments.values()
+    x,y = (x,y) if len(x) > len(y) else (y,x)
+    image_cartesian = list(itertools.product(x, y))
+    image_sets = []
+    for i in image_cartesian:
+        if i[0][0]["Segment"] >= i[1]["Segment"]:
+            image_sets.append(i) 


How was this tested? There is some difference between the example I gave in the issue and how it would be implemented here. Just want to make sure it is working.

So I added some print statements with the old image_sets and image_sets after this for loop for the two areas Lynn mentioned in here (I can point you to the data areas too) and the results are attached as image_set_sort_before.txt and image_set_sort_after.txt. It looks like both examples show the removal of a redundant pair, going from (1,1), (1,2), (2,1), (2,2) to (1,1), (2,1), (2,2).

lwellerastro · 2024-07-31T18:42:57Z

The updates were tested against post #5489 but did not produce a network when it should have. See comments there.

github-actions · 2024-08-02T20:48:11Z

The build and test suite have started for your pull request. View build logs for results.

github-actions · 2024-08-19T15:35:48Z

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5565".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

github-actions · 2024-08-19T15:46:51Z

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5565".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

github-actions · 2024-08-19T15:49:44Z

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5565".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

github-actions · 2024-08-20T00:42:25Z

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5565".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

github-actions · 2024-08-20T21:58:17Z

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5565".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

github-actions · 2024-09-12T19:57:13Z

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5565".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

Kelvinrr · 2024-09-16T17:46:40Z

~~Lynn tested this locally and seem happy with the changes so I think this is good to go~~

False alarm, Lynn still wants to test some more

lwellerastro · 2024-10-04T22:11:43Z

Despite observations made via open post #5540, I think the latest version of the script is worth releasing. It's a great improvement over prior versions and further attempts to find the most suitable use cases will be easier for a user to try with the updated script and xml (containing reference to the new workdir parameter). It's a little difficult testing because of the xml disconnect.

I think moving the segmented cub files from the working directory to /tmp (really not feasible due to NAC file sizes) or a specified workdir needs to be reverted, but that needs to be a new post. I think there is still an issue with creating a final network in some cases, but that needs another look and a new post.

Barring running the script on very large dataset with images having many, many overlaps, I think the script is doing what it should better than before and could be useful for specific use cases. It would be nice to have access to the updates in the latest release if you all agree.

github-actions · 2024-11-04T18:53:44Z

The build and test suite have started for your pull request.

To view your build log, please reference the build with source version: "PR_5565".

Additionally, check the latest "dev" source version to identify existing test failures. Please note that you are not responsible for the test failures that exist on both your PR and the dev branch.

chkim-usgs · 2024-11-04T21:14:23Z

@Kelvinrr Build looks good, test failures match dev's.

chkim-usgs added 2 commits July 29, 2024 14:25

Fix path issue, reduce redundant overlaps

47e9b91

Added comment

1e3b7a2

Kelvinrr reviewed Jul 30, 2024

View reviewed changes

chkim-usgs added 2 commits July 30, 2024 13:45

Remove old image_sets

5d7776b

Add check for match_segment_n

a31e669

Revert overlap changes

698afc6

updated script to improve times and fix bugs

238cdb4

Kelvinrr force-pushed the find_features_fix branch from 03bd425 to 238cdb4 Compare August 19, 2024 15:46

added workdir param

3ef67ab

made sure file are cleaned up when process fails

3ec2eb7

removed bug

df3af20

fixes to tolist

906ee48

Trigger build

bb65769

Kelvinrr approved these changes Nov 4, 2024

View reviewed changes

Kelvinrr merged commit 437e334 into DOI-USGS:dev Nov 4, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

findFeaturesSegment.py script fixes #5565

findFeaturesSegment.py script fixes #5565

chkim-usgs commented Jul 29, 2024

Kelvinrr Jul 30, 2024

chkim-usgs Jul 30, 2024

Kelvinrr Jul 30, 2024

Kelvinrr Jul 30, 2024

chkim-usgs Jul 30, 2024

lwellerastro commented Jul 31, 2024

github-actions bot commented Aug 2, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 20, 2024

github-actions bot commented Aug 20, 2024

github-actions bot commented Sep 12, 2024

Kelvinrr commented Sep 16, 2024 •

edited

Loading

lwellerastro commented Oct 4, 2024

github-actions bot commented Nov 4, 2024

chkim-usgs commented Nov 4, 2024

findFeaturesSegment.py script fixes #5565

findFeaturesSegment.py script fixes #5565

Conversation

chkim-usgs commented Jul 29, 2024

Description

Related Issue

How Has This Been Validated?

Types of changes

Checklist:

Licensing

Kelvinrr Jul 30, 2024

Choose a reason for hiding this comment

chkim-usgs Jul 30, 2024

Choose a reason for hiding this comment

Kelvinrr Jul 30, 2024

Choose a reason for hiding this comment

Kelvinrr Jul 30, 2024

Choose a reason for hiding this comment

chkim-usgs Jul 30, 2024

Choose a reason for hiding this comment

lwellerastro commented Jul 31, 2024

github-actions bot commented Aug 2, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 19, 2024

github-actions bot commented Aug 20, 2024

github-actions bot commented Aug 20, 2024

github-actions bot commented Sep 12, 2024

Kelvinrr commented Sep 16, 2024 • edited Loading

lwellerastro commented Oct 4, 2024

github-actions bot commented Nov 4, 2024

chkim-usgs commented Nov 4, 2024

Kelvinrr commented Sep 16, 2024 •

edited

Loading