added findfeaturesSegment script #5091
Conversation
isis/scripts/findfeaturesSegment
Outdated
| new_params["networkid"] = og_networkid + f"_match_segment{match_segment_n}_from_segment{from_segment_n}" | ||
| new_params["pointid"] = og_pointid + f"_match_segment{match_segment_n}_from_segment{from_segment_n}" | ||
| new_params["onet"] = f"{og_onet.parent/og_onet.stem}.match_segment{match_segment_n}.from_segment{from_segment_n}.net" | ||
| new_params["tolist"] = f"{og_tolist.parent/og_tolist.stem}.match_segment{match_segment_n}.from_segment{from_segment_n}.lis" |
There was a problem hiding this comment.
These should probably be changed to include the image names, not just match/from and segment number
isis/scripts/findfeaturesSegment
Outdated
| def parse_parameters(): | ||
| # Use a regular expression to extract the parameter name and value from each | ||
| # key-value pair in the parameter string. | ||
| pattern = r'(\w+)=(\S+)' | ||
|
|
||
| # Create a dictionary to store the parameter names and values. | ||
| params = {} | ||
| for arg in sys.argv[1:]: | ||
| # Extract the parameter name and value from the argument. | ||
| matches = re.findall(pattern, arg) | ||
| for match in matches: | ||
| name = match[0] |
There was a problem hiding this comment.
kinda ghetto for now, but if we can expose ISIS's UI objects into python, we can write this in a way that is indistinguishable from a C++ ISIS app using Python (I think)
|
I'm running some tests on this now and seeing that the pointid's are a bit on the long side. and my output is like the following: Would it be possible to drop the "M1316912174RE_cal_echo." at the very least if it is important to keep the "segment' references for tracking? If it could be boiled down to "M1316912174RE_fastsobel_00013_segment#" that would be ideal. Edit - this request would also apply to the some of the intermediate output files which have similarly extremely long names: M1316912174RE_cubes_ff.M1316912174RE_cal_echo.segment1.from_images_segment1.lis |
|
I'm having a hard time telling what's going on while with my cluster jobs. My findfeaturesSegment command line includes I need to know in order to ask for the appropriate amount of threads and most importantly memory for a single image findfeatures job execution. This is particularly important when there are limited resources, numerous images to work on (jobs) and multiple people likely to do so at the same time. |
|
I'm getting I believe there are "overlapping" segments that are too slim for findfeatures to work with so it dies. There is no easy way to know what image segment in the fromlist is creating the problem for the match image so it just quits and no images get worked on (a findfeatures problem that is already in another post). The findfeatures workflow starts by passing a config file to isisminer which evaluates the overlap ratios of the match and the images contained in its overlaps.csv file. Overlap ratios that are too small are not allowed to go into the findfeatures fromlist. This is for the whole images. The findfeaturesSegment script seems to evaluate if the match and from segment overlap, but it does not seem to be evaluating if those overlaps as being suitable for the program. This may be why 13 of the 18 jobs (images) I sent to cluster have not finished after 30+ minutes and all seem to be stuck on something, but I can't really tell what is going yet. I don't have any completed networks to look at yet and I'm having trouble tracing some things because issues with having to write to a single print.prt and information not being captured where I usually expect. I can point someone to the data I'm looking at (on our scratch directory) when necessary via chat or email. |
|
I would very much like to help debug some of the problems I am experiencing, but I am having an especially difficult time going through my slurm stdout log file because of the enormous amount of output and seemingly numerous calls to findfeatures. I have no useful print file to look at, and in the worst failure cases (like the example below) there is no findfeatures log file to look through. One thing I am observing, is that it seems there are image segments being passed to findfeatures via the fromlist that do not actually overlap the match file segment being operated on. This may be why most of my jobs hung or returned empty matrix errors. None of my test jobs ran to a successful completion and after 3+ hours of doing nothing I killed the 12 jobs that hung. I'm looking at an image that has some output in it's working directory in the form of from_images_segment#.lis and trying to correlate that back to the text in the slurm log file to see if there is anything useful. Maybe I'm not interpreting things properly, but in the following findfeatures command (copied from the slurm output) I have opened the match image and the files in the fromlist in qmos and can clearly see there is a lack of overlaps for one of the segments (green) with the match image (blue). Maybe there is something going on with that aspect of the script? I've removed all of the relevant path information:
contents of fromlist w/ paths removed with comments inline: See qmos footprint screen capture. Like I said, it was difficult parsing through the slurm output file, so maybe I'm misreading something.
|
|
This is in reference to my last set of comments and failures possibly due to segments not overlapping. To the best of my ability, I am going though the script steps manually that precede the call to findfeatures for the 3 segments above. The match image is M156671044LE.lev1.segment1.cub and in the above case, segment 2 images are trying to be matched to it, M1214450277LE.lev1.segment2.cub and M1214450277RE.lev1.segment2.cub. I'll share my steps below and the results that lead to suggesting changes for determining overlaps before creating a segment fromlist for findfeatures. I'm piecing this together based on the bits and pieces I can pull from what is going to the slurm standard out file since none of it is properly captured to screen or print file since I have to disable those things in my Preference file in order to get the script to run. There may be flaws below due to the limited information. There are a number of temporary files created via script that I am attempting to mimic in my steps below. The filenames are my own. Best I can tell, the script is evaluating the basic Group=Results output from overlapstats to determine if there are overlaps in the input file: This is not especially useful in this case because it does not specify who overlaps who. Notice that I included Sorry for the length of these files. The output shows in the second line that our match segment file of interest M156671044LE.lev1.segment1.cub only overlaps M1214450277RE.lev1.segment2.cub and not the other segment2 image. The first line shows that the two segment2 images overlap each other. M1214450277LE.lev1.segment2.cub should NOT be included in the findfeatures fromlist since it does not overlap the match file we are working on. This is no doubt the sort of thing that will make findfeatures choke. I think the script needs to be reworked to determine the properly overlapping segments to be passed to the findfeatures runs. I further think there needs to be some inspection and filtering on the contents of the detailed overlap statistics to determine if the overlap is large enough to pass to findfeatures. An overlap ratio would be more useful than the thickness or area information being captured by overlapstats which are notoriously difficult to work with. That said, I think setting that value needs to come from the user, not the script since the user will be more familiar with the data. Until having the opportunity to research a cutoff I can't recommend a cutoff for either statistic for exclusion from a fromlist. |
|
Something just occurred to me that might be another issue for this script, but I'm not sure and won't until I am able to run the script. In the links above and through personal communication, @Kelvinrr said the script is parallelized/threaded(?) which may be a problem if that means findfeatures is running multiple instances in the same directory on the same match and fromlist images. Findfeatures is explicitly run in separate match image directories for file management reasons, but also because it reads train (froml) and query (match) images to memory and sometimes to disk (when using the rendered image option) while finding and matching interesting points. I'm probably not articulating this properly and may even have the concepts ill conceived, but actual experience has demonstrated that running multiple instances of findfeatures on multiple images that are involved with not only building up their own network but also included as a fromlist image in other networks (the norm), and doing so all in the same directory, will result scrambled, incomplete, and often undesired results. Separating out the findfeatures runs into individual image network directories fixes this. Again, probably not describing this quite right, but when all actions take place in the same directory, the query (match) and trainer (froml) image overlaps are going to the feature detection algorithm in a projected form via memory (I use fastgeom=true; though I might not be interpreting this aspect properly). This is ok if there is one run for a match with multiple froms - the froms are rotated and scaled to match the match image. And when rendered have names like M1214450277RE.lev1.train.png and M156671044LE.lev1.query.png, which mimics the input filenames. But what happens when there are multiple segments running around and multiple runs of findfeatures occurring at once in the same directory? Match.segment1 passed to findfeatures along with From.segment2 at the same time as Match.segment1 From.segment1 could be a problem. I just don't know, but I think it needs to be thought about for a second if the answer is not obvious. I understand and appreciate that running things in parallel will make the work go quicker, but it could be a problem for this application. If @KrisBecker has any time and/or interest in commenting, that would be greatly appreciated, but understood if he chooses not to considering he is not directly involved with this particular work. I would only ask if the concept of running multiple findfeatures concurrently in the same directory on the same images might pose a problem. Thanks. |
|
@lwellerastro sorry you are having so much trouble. There is a lot going on here and the cluster may be contributing to the problems to some degree (i.e., mostly due to job constraints, perhaps). After reading through your posts, I think there are a few things that are contributing to the issues you are seeing. findfeatures's FastGeom algorithm is quite challenged with the very small overlapping images. The FastGeom::Grid algorithm will iterate way too long trying to construct a perspective transform from common ground points, which are computed from a priori SPICE. The very small sliver/corner of overlap shown in your qmos graphic is exactly the kind of case that will cause findfeatures to iterate seemingly forever. It will eventually fail, killing the job, because it can't get enough common points to create the transformation matrix in FastGeom. And ofttimes, when it does get enough points, it creates a very bad matrix leading to the matrix inversion issue. Threading is likely contributing to some of your problems, particularly if you are running on a cluster that (typically) manages memory and CPU utilization. If its true that this findfeature segmentation script is threaded, and you are requesting threads in findfeatures, the cluster itself could impose limiting that resource such that it will kill the job when you start requesting more threads. It seems to make sense that essentially, if you are running the segmentation on the cluster, which is threaded, it is using all the allocated/specified threads. When a findfeatures run is dispatched on the cluster, it will only get one CPU (or none, which causes a suspension or the job is killed?). And if FastGeom is in its ground point collection loop, it will not finish any time soon, that's for sure. I suggest that if the images are nearly spatially consistent, you may be able to forego using FastGeom. If you do that, be sure to use matching algorithms that are scale and rotation invariant. For cluster management, it might be nice to have the segmenting script submit the individual findfeatures jobs back to the cluster for each segment so that threading is not contentious/competing for the same resource. To answer your specific question if running findfeatures in the same directory is a problem: the only issue I think might be a problem is if you are requesting to save the FastGeom'ed images as PNGs. This could cause the same filenames to be generated from like-named input files and potentially cause file I/O collisions. Finally, I have a long standing findfeatures PR waiting for me to finalize that addresses many of these issues. These modifications greatly improve the FastGeom::Grid algorithm by providing indexed looping ranges, adds a new one shot FastGeom::Radial algorithm, won't fail when sufficent common ground points cannot be found (writes failed image names to a file) and resolves the matrix inversion issues. I am working to finish that PR ASAP - in the next week - that should help with some of these problems. |
|
Thanks for the thoughtful and insightful comments @KrisBecker. There is a lot here to digest and consider and we appreciate you taking the time to provide such useful feedback. |
|
Ping this for a review. It's really important that we get this code merged as soon as we can. |
|
pinged some people I think are appropriate specifically. |
|
@Kelvinrr I'm about 90% of the way through this review, but I don't have a lot of experience with the swig interface files, so I'm moving slowly. I expect to be done with the review in the next couple hours. |
isis/python_bindings/apps/findFeaturesSegment/findfeaturesSegment.py
Outdated
Show resolved
Hide resolved
|
Gonna add some quick tests and I think this might be good for re-review. |
|
This should be good for re-review |
|
Code changes look good, but this is failing a few tests that are passing on dev. Is that expected? Looks like: |
|
@AustinSanders these pass locally, so maybe a red herring? |
| def test_module_import(): | ||
| name = 'astroset' | ||
| if name in sys.modules: | ||
| print(f"{name!r} already in sys.modules") | ||
| assert (spec := importlib.util.find_spec(name)) is not None | ||
|
|
||
| def test_check_pip_install(): | ||
| package_name = "astroset" | ||
| try: | ||
| # Check if the package is installed by running the "pip show" command | ||
| result = subprocess.check_output(['pip', 'show', package_name]) | ||
| package_info = result.decode('utf-8') | ||
|
|
||
| # Parse the package information to check if it contains the package name | ||
| assert(package_name.lower() in package_info.lower()) | ||
|
|
||
| except subprocess.CalledProcessError: | ||
| assert False | ||
|
|
||
| def test_app_install(): | ||
| app_name = 'findFeaturesSegment.py' | ||
| try: | ||
| # Check if the package is installed by running the "pip show" command | ||
| result = subprocess.check_output([app_name, '-H']) | ||
| help_string = result.decode('utf-8') | ||
|
|
||
| # Parse the package information to check if it contains the package name | ||
| assert("from" in help_string.lower()) | ||
| assert("fromlist" in help_string.lower()) | ||
|
|
||
| except subprocess.CalledProcessError: | ||
| assert False |
There was a problem hiding this comment.
some simple install scripts
| cd $WORKSPACE/isis/python_bindings/tests | ||
| pytest . |
There was a problem hiding this comment.
added the tests to jenkins
isis/python_bindings/CMakeLists.txt
Outdated
| install(FILES ${ASTROSET_APP_XML_FILES} DESTINATION ${CMAKE_INSTALL_PREFIX}/bin/xml/) | ||
|
|
||
| # Setup to run setup tools on install | ||
| install(CODE "execute_process(COMMAND $ENV{CONDA_PREFIX}/bin/python ${CMAKE_CURRENT_BINARY_DIR}/setup.py install --single-version-externally-managed --prefix=${CMAKE_INSTALL_PREFIX} --record=record.txt |
There was a problem hiding this comment.
now using the up to date way of installing
|
new tests pass and seem to match dev, I think this is good for re-review @AustinSanders |
|
I really look forward to using this @Kelvinrr !! |
Description
Adds a script that splits images into segments and runs findfeatures on them.
Related Issue
#4396
How Has This Been Validated?
Ran in a jupyter notebook and had the resulting network reviewed by @lwellerastro
Still needs more adjustments so more changes incoming.
Types of changes
Checklist:
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: