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

Jump to bottom

Interface refactor #343

Closed
wants to merge 3 commits into from
Closed

Interface refactor #343

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
07db050
Example refactoring
jeremykubica Sep 19, 2023
16f58e4
Split up common.h
jeremykubica Sep 19, 2023
b1ac016
Splitting bindings into multiple files.
DinoBektesevic Sep 19, 2023
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
89 changes: 85 additions & 4 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,90 @@ set(CMAKE_CXX_STANDARD 11)

include_directories(
include/
src/kbmod/include/
Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, a bit strange to have two include directories for the same code. Python doesn't care about the top-level include dir, so do you want to merge it all all the way down? Optionally we could adopt the more classic C++ way of project organizing where we split up our headers into include and implementation into src. That's how afw did it but I'm not sure what the implicit tradeoffs of doing it like that are. It definitely looks a lot more complicated.

With that also, maybe you could explain to me, here or in a meeting, what are the traditional structures used in C++ projects to manage dependencies? The nvidia headers of error declarations - ok whatevs. But including the whole fitsio.h and hoping it exists out there in the universe somewhere has been the weirdest thing to me.

src/kbmod/image_base/
)

# --------------------------------------------------
# Build the image_base library and python package --
# --------------------------------------------------

add_library(image_base_lib STATIC
src/kbmod/image_base/ImageStack.h
src/kbmod/image_base/ImageStack.cpp
src/kbmod/image_base/LayeredImage.h
src/kbmod/image_base/LayeredImage.cpp
src/kbmod/image_base/PointSpreadFunc.h
src/kbmod/image_base/PointSpreadFunc.cpp
src/kbmod/image_base/RawImage.h
src/kbmod/image_base/RawImage.cpp
)

pybind11_add_module(image_base MODULE
src/kbmod/image_base/bindings.cpp
)

set_target_properties(image_base PROPERTIES
CXX_VISIBILITY_PRESET "hidden"
PREFIX "${PYTHON_MODULE_PREFIX}"
SUFFIX "${PYTHON_MODULE_EXTENSION}"
)

if(ipo_supported)
set_property(TARGET image_base PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
set_property(TARGET image_base_lib PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
endif()

target_compile_options(image_base PRIVATE $<$<COMPILE_LANGUAGE:CXX>:
-O3
-fvisibility=hidden
-fopenmp
>)
target_compile_options(image_base_lib PRIVATE $<$<COMPILE_LANGUAGE:CXX>:
-O3
-fvisibility=hidden
-fopenmp
>)

target_link_libraries(image_base PRIVATE
${CFITSIO_LIBRARY}
-lgomp
)

target_link_libraries(image_base_lib PRIVATE
${CFITSIO_LIBRARY}
-lgomp
)

# If we have CUDA, build the kernel libraries and link them in as well.
if(HAVE_CUDA)
message(STATUS "Building image_base CUDA Libraries")
add_library(image_base_cu STATIC
src/kbmod/image_base/image_kernels.cu
)

set_target_properties(image_base_cu PROPERTIES
POSITION_INDEPENDENT_CODE ON
CUDA_VISIBILITY_PRESET "hidden"
CUDA_SEPARABLE_COMPILATION ON
CUDA_RESOLVE_DEVICE_SYMBOLS ON
PREFIX "${PYTHON_MODULE_PREFIX}"
SUFFIX "${PYTHON_MODULE_EXTENSION}"
)
if(ipo_supported)
set_property(TARGET image_base_cu PROPERTY INTERPROCEDURAL_OPTIMIZATION TRUE)
endif()

target_link_libraries(image_base PRIVATE image_base_cu)
target_link_libraries(image_base_lib PRIVATE image_base_cu)
else()
message(STATUS "Skipping image_base CUDA Libraries")
endif()


# --------------------------------------------------
# Build the search python package ------------------
# --------------------------------------------------

# Create the python module via pybind11.
pybind11_add_module(search MODULE
Expand All @@ -57,16 +139,15 @@ target_compile_options(search PRIVATE $<$<COMPILE_LANGUAGE:CXX>:
>)

target_link_libraries(search PRIVATE
image_base_lib
${CFITSIO_LIBRARY}
-lgomp
)


# If we have CUDA, build the kernel libraries and link them in as well.
if(HAVE_CUDA)
message(STATUS "Building CUDA Libraries")
message(STATUS "Building search CUDA Libraries")
add_library(searchcu STATIC
src/kbmod/search/image_kernels.cu
src/kbmod/search/kernels.cu
)

Expand All @@ -84,5 +165,5 @@ if(HAVE_CUDA)

target_link_libraries(search PRIVATE searchcu)
else()
message(STATUS "Skipping CUDA Libraries")
message(STATUS "Skipping search CUDA Libraries")
endif()
4 changes: 2 additions & 2 deletions src/kbmod/search/ImageStack.cpp → src/kbmod/image_base/ImageStack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include "ImageStack.h"

namespace search {
namespace image_base {

ImageStack::ImageStack(const std::vector<std::string>& filenames, const std::vector<PointSpreadFunc>& psfs) {
verbose = true;
Expand Down Expand Up @@ -134,4 +134,4 @@ void ImageStack::createGlobalMask(int flags, int threshold) {
}
}

} /* namespace search */
} /* namespace image_base */
7 changes: 5 additions & 2 deletions src/kbmod/search/ImageStack.h → src/kbmod/image_base/ImageStack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
#include <list>
#include <iostream>
#include <stdexcept>

#include "common.h"
#include "LayeredImage.h"
#include "RawImage.h"

namespace search {
namespace image_base {

class ImageStack {
public:
Expand Down Expand Up @@ -65,6 +68,6 @@ class ImageStack {
bool verbose;
};

} /* namespace search */
} /* namespace image_base */

#endif /* IMAGESTACK_H_ */
4 changes: 2 additions & 2 deletions src/kbmod/search/LayeredImage.cpp → src/kbmod/image_base/LayeredImage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include "LayeredImage.h"

namespace search {
namespace image_base {

LayeredImage::LayeredImage(std::string path, const PointSpreadFunc& psf) : psf(psf), psfSQ(psf) {
psfSQ.squarePSF();
Expand Down Expand Up @@ -244,4 +244,4 @@ RawImage LayeredImage::generatePhiImage() {
return result;
}

} /* namespace search */
} /* namespace image_base */
7 changes: 4 additions & 3 deletions src/kbmod/search/LayeredImage.h → src/kbmod/image_base/LayeredImage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
#include <random>
#include <assert.h>
#include <stdexcept>
#include "RawImage.h"

#include "common.h"
#include "RawImage.h"

namespace search {
namespace image_base {

class LayeredImage {
public:
Expand Down Expand Up @@ -97,6 +98,6 @@ class LayeredImage {
RawImage variance;
};

} /* namespace search */
} /* namespace image_base */

#endif /* LAYEREDIMAGE_H_ */
4 changes: 2 additions & 2 deletions src/kbmod/search/PointSpreadFunc.cpp → src/kbmod/image_base/PointSpreadFunc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include "PointSpreadFunc.h"

namespace search {
namespace image_base {

PointSpreadFunc::PointSpreadFunc(float stdev) {
width = stdev;
Expand Down Expand Up @@ -140,4 +140,4 @@ std::string PointSpreadFunc::printPSF() {
return ss.str();
}

} /* namespace search */
} /* namespace image_base */
5 changes: 3 additions & 2 deletions src/kbmod/search/PointSpreadFunc.h → src/kbmod/image_base/PointSpreadFunc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
#include <pybind11/numpy.h>
#include <pybind11/stl.h>
#endif

#include "common.h"

namespace search {
namespace image_base {

class PointSpreadFunc {
public:
Expand Down Expand Up @@ -62,6 +63,6 @@ class PointSpreadFunc {
int radius;
};

} /* namespace search */
} /* namespace image_base */

#endif /* POINTSPREADFUNC_H_ */
4 changes: 2 additions & 2 deletions src/kbmod/search/RawImage.cpp → src/kbmod/image_base/RawImage.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#include "RawImage.h"

namespace search {
namespace image_base {

#ifdef HAVE_CUDA
// Performs convolution between an image represented as an array of floats
Expand Down Expand Up @@ -631,4 +631,4 @@ RawImage createMeanImage(const std::vector<RawImage>& images) {
return result;
}

} /* namespace search */
} /* namespace image_base */
6 changes: 3 additions & 3 deletions src/kbmod/search/RawImage.h → src/kbmod/image_base/RawImage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#include "common.h"
#include "PointSpreadFunc.h"

namespace search {
namespace image_base {

class RawImage {
public:
Expand Down Expand Up @@ -136,6 +136,6 @@ RawImage createMedianImage(const std::vector<RawImage>& images);
RawImage createSummedImage(const std::vector<RawImage>& images);
RawImage createMeanImage(const std::vector<RawImage>& images);

} /* namespace search */

} /* namespace image_base */
#endif /* RAWIMAGE_H_ */
Loading
Loading