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.

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

Interface refactor #343

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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()
20 changes: 10 additions & 10 deletions src/kbmod/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
# lazy import analysis to arrest
# loading large libraries in them
# import the filters subdirectory
from . import (
analysis,
analysis_utils,
file_utils,
filters,
image_info,
jointfit_functions,
result_list,
run_search,
)
#from . import (
# analysis,
# analysis_utils,
# file_utils,
# filters,
# image_info,
# jointfit_functions,
# result_list,
# run_search,
#)
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 */
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_ */
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

#include "LayeredImage.h"

namespace search {
namespace image_base {

LayeredImage::LayeredImage(std::string path, const PointSpreadFunc& psf) : psf(psf), psfSQ(psf) {
psfSQ.squarePSF();
psfSQ.square_psf();

int fBegin = path.find_last_of("/");
int fEnd = path.find_last_of(".fits") - 4;
Expand Down Expand Up @@ -46,7 +46,7 @@ LayeredImage::LayeredImage(const RawImage& sci, const RawImage& var, const RawIm
throw std::runtime_error("Science and Mask layers are not the same size.");

// Set the remaining variables.
psfSQ.squarePSF();
psfSQ.square_psf();

// Copy the image layers.
science = sci;
Expand All @@ -64,7 +64,7 @@ LayeredImage::LayeredImage(std::string name, int w, int h, float noiseStDev, flo
fileName = name;
width = w;
height = h;
psfSQ.squarePSF();
psfSQ.square_psf();

std::vector<float> rawSci(width * height);
std::random_device r;
Expand All @@ -85,14 +85,14 @@ LayeredImage::LayeredImage(std::string name, int w, int h, float noiseStDev, flo
void LayeredImage::setPSF(const PointSpreadFunc& new_psf) {
psf = new_psf;
psfSQ = new_psf;
psfSQ.squarePSF();
psfSQ.square_psf();
}

void LayeredImage::addObject(float x, float y, float flux) {
const std::vector<float>& k = psf.getKernel();
int dim = psf.getDim();
float initialX = x - static_cast<float>(psf.getRadius());
float initialY = y - static_cast<float>(psf.getRadius());
const std::vector<float>& k = psf.get_kernel();
int dim = psf.get_dim();
float initialX = x - static_cast<float>(psf.get_radius());
float initialY = y - static_cast<float>(psf.get_radius());

int count = 0;
for (int i = 0; i < dim; ++i) {
Expand Down Expand Up @@ -244,4 +244,4 @@ RawImage LayeredImage::generatePhiImage() {
return result;
}

} /* namespace search */
} /* namespace image_base */
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_ */
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 @@ -40,7 +40,7 @@ PointSpreadFunc::PointSpreadFunc(float stdev) {
kernel.push_back(current);
}
}
calcSum();
calc_sum();
}

// Copy constructor.
Expand Down Expand Up @@ -83,9 +83,9 @@ PointSpreadFunc& PointSpreadFunc::operator=(PointSpreadFunc&& other) {
}

#ifdef Py_PYTHON_H
PointSpreadFunc::PointSpreadFunc(pybind11::array_t<float> arr) { setArray(arr); }
PointSpreadFunc::PointSpreadFunc(pybind11::array_t<float> arr) { set_array(arr); }

void PointSpreadFunc::setArray(pybind11::array_t<float> arr) {
void PointSpreadFunc::set_array(pybind11::array_t<float> arr) {
pybind11::buffer_info info = arr.request();

if (info.ndim != 2)
Expand All @@ -106,24 +106,24 @@ void PointSpreadFunc::setArray(pybind11::array_t<float> arr) {
radius = dim / 2; // Rounds down
sum = 0.0;
kernel = std::vector<float>(pix, pix + dim * dim);
calcSum();
calc_sum();
width = 0.0;
}
#endif

void PointSpreadFunc::calcSum() {
void PointSpreadFunc::calc_sum() {
sum = 0.0;
for (auto& i : kernel) sum += i;
}

void PointSpreadFunc::squarePSF() {
void PointSpreadFunc::square_psf() {
for (float& i : kernel) {
i = i * i;
}
calcSum();
calc_sum();
}

std::string PointSpreadFunc::printPSF() {
std::string PointSpreadFunc::print() {
std::stringstream ss;
ss.setf(std::ios::fixed, std::ios::floatfield);
ss.precision(3);
Expand All @@ -140,4 +140,4 @@ std::string PointSpreadFunc::printPSF() {
return ss.str();
}

} /* namespace search */
} /* namespace image_base */
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,18 @@
#include <sstream>
#include <vector>
#include <stdexcept>
#ifdef Py_PYTHON_H
#include <pybind11/pybind11.h>
#include <pybind11/numpy.h>
#include <pybind11/stl.h>
#endif
#include "common.h"

namespace search {

class PointSpreadFunc {
public:
namespace image_base {
class PointSpreadFunc {
public:
PointSpreadFunc(float stdev);
PointSpreadFunc(const PointSpreadFunc& other); // Copy constructor
PointSpreadFunc(PointSpreadFunc&& other); // Move constructor
#ifdef Py_PYTHON_H
PointSpreadFunc(pybind11::array_t<float> arr);
void setArray(pybind11::array_t<float> arr);
void set_array(pybind11::array_t<float> arr);
#endif
virtual ~PointSpreadFunc(){};

Expand All @@ -40,28 +35,27 @@ class PointSpreadFunc {
PointSpreadFunc& operator=(PointSpreadFunc&& other); // Move assignment

// Getter functions (inlined)
float getStdev() const { return width; }
float getSum() const { return sum; }
float getValue(int x, int y) const { return kernel[y * dim + x]; }
int getDim() const { return dim; }
int getRadius() const { return radius; }
int getSize() const { return kernel.size(); }
const std::vector<float>& getKernel() const { return kernel; };
float* kernelData() { return kernel.data(); }
float get_stdev() const { return width; }
float get_sum() const { return sum; }
float get_value(int x, int y) const { return kernel[y * dim + x]; }
int get_dim() const { return dim; }
int get_radius() const { return radius; }
int get_size() const { return kernel.size(); }
const std::vector<float>& get_kernel() const { return kernel; };
float* data() { return kernel.data(); }

// Computation functions.
void calcSum();
void squarePSF();
std::string printPSF();
void calc_sum();
void square_psf();
std::string print();

private:
private:
std::vector<float> kernel;
float width;
float sum;
int dim;
int radius;
};

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

#endif /* POINTSPREADFUNC_H_ */
Loading
Loading