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

Windows Tests #1109

Merged
merged 18 commits into from
Feb 22, 2022
Merged
Show file tree
Hide file tree
Changes from 14 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
17 changes: 16 additions & 1 deletion .github/workflows/build-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,21 @@ jobs:
cmake --build build -j 4 --config ${{ matrix.build_type }} --target gtsam
cmake --build build -j 4 --config ${{ matrix.build_type }} --target gtsam_unstable
cmake --build build -j 4 --config ${{ matrix.build_type }} --target wrap

# Run GTSAM tests
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.base
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.base_unstable
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.basis
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.discrete
#cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.geometry
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.inference
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.linear
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.navigation
#cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.nonlinear
#cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.sam
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.sfm
#cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.slam
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.symbolic

# Run GTSAM_UNSTABLE tests
cmake --build build -j 4 --config ${{ matrix.build_type }} --target check.base_unstable

2 changes: 1 addition & 1 deletion DEVELOP.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ For example:
```cpp
class GTSAM_EXPORT MyClass { ... };

GTSAM_EXPORT myFunction();
GTSAM_EXPORT return_type myFunction();
```

More details [here](Using-GTSAM-EXPORT.md).
1 change: 1 addition & 0 deletions Using-GTSAM-EXPORT.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ To create a DLL in windows, the `GTSAM_EXPORT` keyword has been created and need
* At least one of the functions inside that class is declared in a .cpp file and not just the .h file.
* You can `GTSAM_EXPORT` any class it inherits from as well. (Note that this implictly requires the class does not derive from a "header-only" class. Note that Eigen is a "header-only" library, so if your class derives from Eigen, _do not_ use `GTSAM_EXPORT` in the class definition!)
3. If you have defined a class using `GTSAM_EXPORT`, do not use `GTSAM_EXPORT` in any of its individual function declarations. (Note that you _can_ put `GTSAM_EXPORT` in the definition of individual functions within a class as long as you don't put `GTSAM_EXPORT` in the class definition.)
4. For template specializations, you need to add `GTSAM_EXPORT` to each individual specialization.

## When is GTSAM_EXPORT being used incorrectly
Unfortunately, using `GTSAM_EXPORT` incorrectly often does not cause a compiler or linker error in the library that is being compiled, but only when you try to use that DLL in a different library. For example, an error in `gtsam/base` will often show up when compiling the `check_base_program` or the MATLAB wrapper, but not when compiling/linking gtsam itself. The most common errors will say something like:
Expand Down
4 changes: 4 additions & 0 deletions cmake/GtsamBuildTypes.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ if(MSVC)
/wd4267 # warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
)

add_compile_options(/wd4005)
add_compile_options(/wd4101)
add_compile_options(/wd4834)

endif()

# Other (non-preprocessor macros) compiler flags:
Expand Down
7 changes: 1 addition & 6 deletions gtsam/basis/Basis.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Matrix kroneckerProductIdentity(const Weights& w) {

/// CRTP Base class for function bases
template <typename DERIVED>
class GTSAM_EXPORT Basis {
class Basis {
public:
/**
* Calculate weights for all x in vector X.
Expand Down Expand Up @@ -497,11 +497,6 @@ class GTSAM_EXPORT Basis {
}
};

// Vector version for MATLAB :-(
static double Derivative(double x, const Vector& p, //
OptionalJacobian</*1xN*/ -1, -1> H = boost::none) {
return DerivativeFunctor(x)(p.transpose(), H);
}
};

} // namespace gtsam
14 changes: 7 additions & 7 deletions gtsam/basis/BasisFactors.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace gtsam {
* @tparam BASIS The basis class to use e.g. Chebyshev2
*/
template <class BASIS>
class GTSAM_EXPORT EvaluationFactor : public FunctorizedFactor<double, Vector> {
class EvaluationFactor : public FunctorizedFactor<double, Vector> {
private:
using Base = FunctorizedFactor<double, Vector>;

Expand Down Expand Up @@ -85,7 +85,7 @@ class GTSAM_EXPORT EvaluationFactor : public FunctorizedFactor<double, Vector> {
* @param M: Size of the evaluated state vector.
*/
template <class BASIS, int M>
class GTSAM_EXPORT VectorEvaluationFactor
class VectorEvaluationFactor
: public FunctorizedFactor<Vector, ParameterMatrix<M>> {
private:
using Base = FunctorizedFactor<Vector, ParameterMatrix<M>>;
Expand Down Expand Up @@ -148,7 +148,7 @@ class GTSAM_EXPORT VectorEvaluationFactor
* where N is the degree and i is the component index.
*/
template <class BASIS, size_t P>
class GTSAM_EXPORT VectorComponentFactor
class VectorComponentFactor
: public FunctorizedFactor<double, ParameterMatrix<P>> {
private:
using Base = FunctorizedFactor<double, ParameterMatrix<P>>;
Expand Down Expand Up @@ -217,7 +217,7 @@ class GTSAM_EXPORT VectorComponentFactor
* where `x` is the value (e.g. timestep) at which the rotation was evaluated.
*/
template <class BASIS, typename T>
class GTSAM_EXPORT ManifoldEvaluationFactor
class ManifoldEvaluationFactor
: public FunctorizedFactor<T, ParameterMatrix<traits<T>::dimension>> {
private:
using Base = FunctorizedFactor<T, ParameterMatrix<traits<T>::dimension>>;
Expand Down Expand Up @@ -269,7 +269,7 @@ class GTSAM_EXPORT ManifoldEvaluationFactor
* @param BASIS: The basis class to use e.g. Chebyshev2
*/
template <class BASIS>
class GTSAM_EXPORT DerivativeFactor
class DerivativeFactor
: public FunctorizedFactor<double, typename BASIS::Parameters> {
private:
using Base = FunctorizedFactor<double, typename BASIS::Parameters>;
Expand Down Expand Up @@ -318,7 +318,7 @@ class GTSAM_EXPORT DerivativeFactor
* @param M: Size of the evaluated state vector derivative.
*/
template <class BASIS, int M>
class GTSAM_EXPORT VectorDerivativeFactor
class VectorDerivativeFactor
: public FunctorizedFactor<Vector, ParameterMatrix<M>> {
private:
using Base = FunctorizedFactor<Vector, ParameterMatrix<M>>;
Expand Down Expand Up @@ -371,7 +371,7 @@ class GTSAM_EXPORT VectorDerivativeFactor
* @param P: Size of the control component derivative.
*/
template <class BASIS, int P>
class GTSAM_EXPORT ComponentDerivativeFactor
class ComponentDerivativeFactor
: public FunctorizedFactor<double, ParameterMatrix<P>> {
private:
using Base = FunctorizedFactor<double, ParameterMatrix<P>>;
Expand Down
4 changes: 2 additions & 2 deletions gtsam/basis/Chebyshev.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace gtsam {
* These are typically denoted with the symbol T_n, where n is the degree.
* The parameter N is the number of coefficients, i.e., N = n+1.
*/
struct Chebyshev1Basis : Basis<Chebyshev1Basis> {
struct GTSAM_EXPORT Chebyshev1Basis : Basis<Chebyshev1Basis> {
using Parameters = Eigen::Matrix<double, -1, 1 /*Nx1*/>;

Parameters parameters_;
Expand Down Expand Up @@ -79,7 +79,7 @@ struct Chebyshev1Basis : Basis<Chebyshev1Basis> {
* functions. In this sense, they are like the sines and cosines of the Fourier
* basis.
*/
struct Chebyshev2Basis : Basis<Chebyshev2Basis> {
struct GTSAM_EXPORT Chebyshev2Basis : Basis<Chebyshev2Basis> {
using Parameters = Eigen::Matrix<double, -1, 1 /*Nx1*/>;

/**
Expand Down
2 changes: 1 addition & 1 deletion gtsam/basis/Fourier.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
namespace gtsam {

/// Fourier basis
class GTSAM_EXPORT FourierBasis : public Basis<FourierBasis> {
class FourierBasis : public Basis<FourierBasis> {
public:
using Parameters = Eigen::Matrix<double, /*Nx1*/ -1, 1>;
using DiffMatrix = Eigen::Matrix<double, /*NxN*/ -1, -1>;
Expand Down
3 changes: 0 additions & 3 deletions gtsam/basis/basis.i
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ class Chebyshev2 {
static Matrix DerivativeWeights(size_t N, double x, double a, double b);
static Matrix IntegrationWeights(size_t N, double a, double b);
static Matrix DifferentiationMatrix(size_t N, double a, double b);

// TODO Needs OptionalJacobian
// static double Derivative(double x, Vector f);
};

#include <gtsam/basis/ParameterMatrix.h>
Expand Down
4 changes: 2 additions & 2 deletions gtsam/discrete/AlgebraicDecisionTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ namespace gtsam {
* TODO: consider eliminating this class altogether?
*/
template <typename L>
class GTSAM_EXPORT AlgebraicDecisionTree : public DecisionTree<L, double> {
class AlgebraicDecisionTree : public DecisionTree<L, double> {
/**
* @brief Default method used by `labelFormatter` or `valueFormatter` when
* printing.
Expand Down Expand Up @@ -127,7 +127,7 @@ namespace gtsam {
return map.at(label);
};
std::function<double(const double&)> op = Ring::id;
this->root_ = this->template convertFrom(other.root_, L_of_M, op);
this->root_ = DecisionTree<L, double>::convertFrom(other.root_, L_of_M, op);
}

/** sum */
Expand Down
2 changes: 1 addition & 1 deletion gtsam/discrete/DiscreteLookupDAG.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class DiscreteBayesNet;
* Inherits from discrete conditional for convenience, but is not normalized.
* Is used in the max-product algorithm.
*/
class DiscreteLookupTable : public DiscreteConditional {
class GTSAM_EXPORT DiscreteLookupTable : public DiscreteConditional {
public:
using This = DiscreteLookupTable;
using shared_ptr = boost::shared_ptr<This>;
Expand Down
2 changes: 1 addition & 1 deletion gtsam/discrete/DiscreteMarginals.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace gtsam {
/**
* A class for computing marginals of variables in a DiscreteFactorGraph
*/
class GTSAM_EXPORT DiscreteMarginals {
class DiscreteMarginals {

protected:

Expand Down
2 changes: 1 addition & 1 deletion gtsam/discrete/DiscreteValues.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ namespace gtsam {
* stores cardinality of a Discrete variable. It should be handled naturally in
* the new class DiscreteValue, as the variable's type (domain)
*/
class DiscreteValues : public Assignment<Key> {
class GTSAM_EXPORT DiscreteValues : public Assignment<Key> {
public:
using Base = Assignment<Key>; // base class

Expand Down
28 changes: 1 addition & 27 deletions gtsam/geometry/Line3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,30 +91,4 @@ Point3 Line3::point(double distance) const {
return rotated_center + distance * R_.r3();
}

Line3 transformTo(const Pose3 &wTc, const Line3 &wL,
OptionalJacobian<4, 6> Dpose, OptionalJacobian<4, 4> Dline) {
Rot3 wRc = wTc.rotation();
Rot3 cRw = wRc.inverse();
Rot3 cRl = cRw * wL.R_;

Vector2 w_ab;
Vector3 t = ((wL.R_).transpose() * wTc.translation());
Vector2 c_ab(wL.a_ - t[0], wL.b_ - t[1]);

if (Dpose) {
Matrix3 lRc = (cRl.matrix()).transpose();
Dpose->setZero();
// rotation
Dpose->block<2, 3>(0, 0) = -lRc.block<2, 3>(0, 0);
// translation
Dpose->block<2, 3>(2, 3) = -lRc.block<2, 3>(0, 0);
}
if (Dline) {
Dline->setIdentity();
(*Dline)(0, 3) = -t[2];
(*Dline)(1, 2) = t[2];
}
return Line3(cRl, c_ab[0], c_ab[1]);
}

}
} // namespace gtsam
Copy link
Member

Choose a reason for hiding this comment

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

Why ?

27 changes: 25 additions & 2 deletions gtsam/geometry/Line3.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace gtsam {
* @addtogroup geometry
* \nosubgrouping
*/
class Line3 {
class GTSAM_EXPORT Line3 {
private:
Rot3 R_; // Rotation of line about x and y in world frame
double a_, b_; // Intersection of line with the world x-y plane rotated by R_
Expand Down Expand Up @@ -146,7 +146,30 @@ class Line3 {
*/
Line3 transformTo(const Pose3 &wTc, const Line3 &wL,
OptionalJacobian<4, 6> Dpose = boost::none,
OptionalJacobian<4, 4> Dline = boost::none);
OptionalJacobian<4, 4> Dline = boost::none) {
Copy link
Member

Choose a reason for hiding this comment

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

I had trouble with definitions in headers. Should be inline? But I don't understand why we can't just export here and leave definition in .cpp?

Rot3 wRc = wTc.rotation();
Rot3 cRw = wRc.inverse();
Rot3 cRl = cRw * wL.R_;

Vector2 w_ab;
Vector3 t = ((wL.R_).transpose() * wTc.translation());
Vector2 c_ab(wL.a_ - t[0], wL.b_ - t[1]);

if (Dpose) {
Matrix3 lRc = (cRl.matrix()).transpose();
Dpose->setZero();
// rotation
Dpose->block<2, 3>(0, 0) = -lRc.block<2, 3>(0, 0);
// translation
Dpose->block<2, 3>(2, 3) = -lRc.block<2, 3>(0, 0);
}
if (Dline) {
Dline->setIdentity();
(*Dline)(0, 3) = -t[2];
(*Dline)(1, 2) = t[2];
}
return Line3(cRl, c_ab[0], c_ab[1]);
}

template<>
struct traits<Line3> : public internal::Manifold<Line3> {};
Expand Down
9 changes: 5 additions & 4 deletions gtsam/geometry/SOn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
namespace gtsam {

template <>
GTSAM_EXPORT void SOn::Hat(const Vector &xi, Eigen::Ref<Matrix> X) {
void SOn::Hat(const Vector &xi, Eigen::Ref<Matrix> X) {
size_t n = AmbientDim(xi.size());
if (n < 2)
throw std::invalid_argument("SO<N>::Hat: n<2 not supported");
Expand All @@ -48,15 +48,14 @@ GTSAM_EXPORT void SOn::Hat(const Vector &xi, Eigen::Ref<Matrix> X) {
}
}

template <> GTSAM_EXPORT Matrix SOn::Hat(const Vector &xi) {
template <> Matrix SOn::Hat(const Vector &xi) {
size_t n = AmbientDim(xi.size());
Matrix X(n, n); // allocate space for n*n skew-symmetric matrix
SOn::Hat(xi, X);
return X;
}

template <>
GTSAM_EXPORT
Vector SOn::Vee(const Matrix& X) {
const size_t n = X.rows();
if (n < 2) throw std::invalid_argument("SO<N>::Hat: n<2 not supported");
Expand Down Expand Up @@ -104,7 +103,9 @@ SOn LieGroup<SOn, Eigen::Dynamic>::between(const SOn& g, DynamicJacobian H1,
}

// Dynamic version of vec
template <> typename SOn::VectorN2 SOn::vec(DynamicJacobian H) const {
template <>
typename SOn::VectorN2 SOn::vec(DynamicJacobian H) const
{
const size_t n = rows(), n2 = n * n;

// Vectorize
Expand Down
6 changes: 5 additions & 1 deletion gtsam/geometry/SOn.h
Original file line number Diff line number Diff line change
Expand Up @@ -358,17 +358,21 @@ Vector SOn::Vee(const Matrix& X);
using DynamicJacobian = OptionalJacobian<Eigen::Dynamic, Eigen::Dynamic>;

template <>
GTSAM_EXPORT
SOn LieGroup<SOn, Eigen::Dynamic>::compose(const SOn& g, DynamicJacobian H1,
DynamicJacobian H2) const;

template <>
GTSAM_EXPORT
SOn LieGroup<SOn, Eigen::Dynamic>::between(const SOn& g, DynamicJacobian H1,
DynamicJacobian H2) const;

/*
* Specialize dynamic vec.
*/
template <> typename SOn::VectorN2 SOn::vec(DynamicJacobian H) const;
template <>
GTSAM_EXPORT
typename SOn::VectorN2 SOn::vec(DynamicJacobian H) const;

/** Serialization function */
template<class Archive>
Expand Down
Loading