Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FindImageOverlap optional threaded computation #4047

Merged
merged 5 commits into from
Oct 2, 2020
Merged

FindImageOverlap optional threaded computation #4047

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e7a0af
Made the threaded calculation option in findimageoverlaps function call
acpaquette Oct 1, 2020
ac3aae3
Fixed type in test file name
acpaquette Oct 1, 2020
d704cea
Merge branch 'dev' of https://github.com/USGS-Astrogeology/ISIS3 into…
acpaquette Oct 1, 2020
6c2036f
Updated change log
acpaquette Oct 1, 2020
d6e7c25
Addressed PR feedback
acpaquette Oct 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions isis/src/base/apps/findimageoverlaps/findimageoverlaps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ using namespace std;

namespace Isis {

void findimageoverlaps(UserInterface &ui, Pvl *log) {
void findimageoverlaps(UserInterface &ui, bool threadedCalculate, Pvl *log) {
acpaquette marked this conversation as resolved.
Show resolved Hide resolved
FileList images(ui.GetFileName("FROMLIST"));

findimageoverlaps(images, ui, log);
findimageoverlaps(images, ui, threadedCalculate, log);
}

void findimageoverlaps(FileList &images, UserInterface &ui, Pvl *log) {
void findimageoverlaps(FileList &images, UserInterface &ui, bool threadedCalculate, Pvl *log) {
// list of sns/filenames sorted by serial number
SerialNumberList serialNumbers(true);

Expand Down Expand Up @@ -47,7 +47,7 @@ namespace Isis {
}

// Now we want the ImageOverlapSet to calculate our overlaps
ImageOverlapSet overlaps(true);
ImageOverlapSet overlaps(true, threadedCalculate);

// Use multi-threading to create the overlaps
overlaps.FindImageOverlaps(serialNumbers,
Expand Down
4 changes: 2 additions & 2 deletions isis/src/base/apps/findimageoverlaps/findimageoverlaps.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
#include "UserInterface.h"

namespace Isis {
extern void findimageoverlaps(UserInterface &ui, Pvl *log=nullptr);
extern void findimageoverlaps(UserInterface &ui, bool threadedCalculate=true, Pvl *log=nullptr);

extern void findimageoverlaps(FileList &images, UserInterface &ui, Pvl *log=nullptr);
extern void findimageoverlaps(FileList &images, UserInterface &ui, bool threadedCalculate=true, Pvl *log=nullptr);
}

#endif
4 changes: 2 additions & 2 deletions isis/src/base/apps/findimageoverlaps/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ using namespace Isis;
void IsisMain() {
UserInterface &ui = Application::GetUserInterface();
Pvl appLog;

try {
findimageoverlaps(ui, &appLog);
findimageoverlaps(ui, true, &appLog);
}
catch (...) {
for (auto grpIt = appLog.beginGroup(); grpIt!= appLog.endGroup(); grpIt++) {
Expand Down
7 changes: 2 additions & 5 deletions isis/src/base/objs/ImageOverlapSet/ImageOverlapSet.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ namespace Isis {
* addition to logging errors.
* @see automaticRegistration.doc
*/
ImageOverlapSet::ImageOverlapSet(bool continueOnError) {
ImageOverlapSet::ImageOverlapSet(bool continueOnError, bool threadedCalculate) {

p_continueAfterError = continueOnError;
p_writtenSoFar = 0;
p_calculatedSoFar = -1;
p_threadedCalculate = false;
p_threadedCalculate = threadedCalculate;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, this was originally forced false here then true later down the line (for reasons? 🤷 ), does just leaving at false or true throughout cause errors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe it will. The control flow in findimageoverlaps doesn't care about the p_threadedCalculate until after being set to true. If this does cause an error somewhere else I am expecting to see it in the jenkins build.

p_snlist = NULL;
}

Expand Down Expand Up @@ -181,9 +181,6 @@ namespace Isis {

p_snlist = &boundaries;

// This will enable using mutexes and spawning threads where necessary.
p_threadedCalculate = true;

FindImageOverlaps(boundaries);

// While our exit condition is not true, call WriteImageOverlaps with the filename.
Expand Down
4 changes: 2 additions & 2 deletions isis/src/base/objs/ImageOverlapSet/ImageOverlapSet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ namespace Isis {
* undefined behavior caused by unlocking an unlocked mutex.
* @history 2017-05-23 Ian Humphrey - Added a tryLock() to FindAllOverlaps to prevent a
* segfault from occuring on OSX with certain data. Fixes #4810.
*
*
*/
class ImageOverlapSet : private QThread {
public:
ImageOverlapSet(bool continueOnError = false);
ImageOverlapSet(bool continueOnError = false, bool threadedCalculate = true);
virtual ~ImageOverlapSet();

void FindImageOverlaps(SerialNumberList &boundaries);
Expand Down
6 changes: 3 additions & 3 deletions ...tests/FuntionalTestsFindImageOverlaps.cpp → ...ests/FunctionalTestsFindImageOverlaps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ TEST_F(ThreeImageNetwork, FunctionalTestFindImageOverlapsNoOverlap) {
Pvl appLog;

try {
findimageoverlaps(options, &appLog);
findimageoverlaps(options, false, &appLog);
FAIL() << "Expected an IException with message: \"No overlaps were found\".";
}
catch(IException &e) {
Expand All @@ -76,7 +76,7 @@ TEST_F(ThreeImageNetwork, FunctionalTestFindImageOverlapTwoImageOverlap) {
FileList images;
images.append(FileName(cube1->fileName()));
images.append(FileName(cube2->fileName()));
findimageoverlaps(images, ui, nullptr);
findimageoverlaps(images, ui, false, nullptr);

// Find all the overlaps between the images in the FROMLIST
// The overlap polygon coordinates are in Lon/Lat order
Expand Down Expand Up @@ -145,7 +145,7 @@ TEST_F(ThreeImageNetwork, FunctionalTestFindImageOverlapFullOverlap) {
FileList images;
images.append(FileName(cube1->fileName()));
images.append(FileName(cube2->fileName()));
findimageoverlaps(images, ui, nullptr);
findimageoverlaps(images, ui, false, nullptr);

// Find all the overlaps between the images in the FROMLIST
// The overlap polygon coordinates are in Lon/Lat order
Expand Down