diff --git a/gtsam/nonlinear/Expression-inl.h b/gtsam/nonlinear/Expression-inl.h index f15ad8575..71ef1d840 100644 --- a/gtsam/nonlinear/Expression-inl.h +++ b/gtsam/nonlinear/Expression-inl.h @@ -189,15 +189,13 @@ T Expression::valueAndDerivatives(const Values& values, template T Expression::traceExecution(const Values& values, - internal::ExecutionTrace& trace, void* traceStorage) const { - return root_->traceExecution(values, trace, - static_cast(traceStorage)); + internal::ExecutionTrace& trace, char* traceStorage) const { + return root_->traceExecution(values, trace, traceStorage); } // Allocate a single block of aligned memory using a unique_ptr. inline std::unique_ptr allocAligned(size_t size) { const size_t alignedSize = (size + internal::TraceAlignment - 1) / internal::TraceAlignment; - std::cerr << size << " : " << alignedSize << '\n'; return std::unique_ptr( new internal::ExecutionTraceStorage[alignedSize]); } @@ -214,7 +212,7 @@ T Expression::valueAndJacobianMap(const Values& values, // with an execution trace, made up entirely of "Record" structs, see // the FunctionalNode class in expression-inl.h internal::ExecutionTrace trace; - T value(this->traceExecution(values, trace, traceStorage.get())); + T value(this->traceExecution(values, trace, reinterpret_cast(traceStorage.get()))); // We then calculate the Jacobians using reverse automatic differentiation (AD). trace.startReverseAD1(jacobians); diff --git a/gtsam/nonlinear/Expression.h b/gtsam/nonlinear/Expression.h index fb8087133..1e977ade1 100644 --- a/gtsam/nonlinear/Expression.h +++ b/gtsam/nonlinear/Expression.h @@ -197,7 +197,7 @@ protected: /// trace execution, very unsafe T traceExecution(const Values& values, internal::ExecutionTrace& trace, - void* traceStorage) const; + char* traceStorage) const; /// brief Return value and derivatives, reverse AD version T valueAndJacobianMap(const Values& values, diff --git a/gtsam/nonlinear/internal/ExpressionNode.h b/gtsam/nonlinear/internal/ExpressionNode.h index dc0cf3764..1f45f7a87 100644 --- a/gtsam/nonlinear/internal/ExpressionNode.h +++ b/gtsam/nonlinear/internal/ExpressionNode.h @@ -110,7 +110,7 @@ public: /// Construct an execution trace for reverse AD virtual T traceExecution(const Values& values, ExecutionTrace& trace, - ExecutionTraceStorage* traceStorage) const = 0; + char* traceStorage) const = 0; }; //----------------------------------------------------------------------------- @@ -146,7 +146,7 @@ public: /// Construct an execution trace for reverse AD T traceExecution(const Values& values, ExecutionTrace& trace, - ExecutionTraceStorage* traceStorage) const override { + char* traceStorage) const override { return constant_; } @@ -199,7 +199,7 @@ public: /// Construct an execution trace for reverse AD T traceExecution(const Values& values, ExecutionTrace& trace, - ExecutionTraceStorage* traceStorage) const override { + char* traceStorage) const override { trace.setLeaf(key_); return values.at(key_); } @@ -276,7 +276,7 @@ public: A1 value1; /// Construct record by calling argument expression - Record(const Values& values, const ExpressionNode& expression1, ExecutionTraceStorage* ptr) + Record(const Values& values, const ExpressionNode& expression1, char* ptr) : value1(expression1.traceExecution(values, trace1, ptr + upAligned(sizeof(Record)))) {} /// Print to std::cout @@ -308,7 +308,7 @@ public: /// Construct an execution trace for reverse AD T traceExecution(const Values& values, ExecutionTrace& trace, - ExecutionTraceStorage* ptr) const override { + char* ptr) const override { assert(reinterpret_cast(ptr) % TraceAlignment == 0); // Create a Record in the memory pointed to by ptr @@ -399,7 +399,7 @@ public: /// Construct record by calling argument expressions Record(const Values& values, const ExpressionNode& expression1, - const ExpressionNode& expression2, ExecutionTraceStorage* ptr) + const ExpressionNode& expression2, char* ptr) : value1(expression1.traceExecution(values, trace1, ptr += upAligned(sizeof(Record)))), value2(expression2.traceExecution(values, trace2, ptr += expression1.traceSize())) {} @@ -427,7 +427,7 @@ public: /// Construct an execution trace for reverse AD, see UnaryExpression for explanation T traceExecution(const Values& values, ExecutionTrace& trace, - ExecutionTraceStorage* ptr) const override { + char* ptr) const override { assert(reinterpret_cast(ptr) % TraceAlignment == 0); Record* record = new (ptr) Record(values, *expression1_, *expression2_, ptr); trace.setFunction(record); @@ -498,6 +498,8 @@ public: // Inner Record Class struct Record: public CallRecordImplementor::dimension> { + EIGEN_MAKE_ALIGNED_OPERATOR_NEW + typename Jacobian::type dTdA1; typename Jacobian::type dTdA2; typename Jacobian::type dTdA3; @@ -513,7 +515,7 @@ public: /// Construct record by calling 3 argument expressions Record(const Values& values, const ExpressionNode& expression1, const ExpressionNode& expression2, - const ExpressionNode& expression3, ExecutionTraceStorage* ptr) + const ExpressionNode& expression3, char* ptr) : value1(expression1.traceExecution(values, trace1, ptr += upAligned(sizeof(Record)))), value2(expression2.traceExecution(values, trace2, ptr += expression1.traceSize())), value3(expression3.traceExecution(values, trace3, ptr += expression2.traceSize())) {} @@ -545,7 +547,7 @@ public: /// Construct an execution trace for reverse AD, see UnaryExpression for explanation T traceExecution(const Values& values, ExecutionTrace& trace, - ExecutionTraceStorage* ptr) const override { + char* ptr) const override { assert(reinterpret_cast(ptr) % TraceAlignment == 0); Record* record = new (ptr) Record(values, *expression1_, *expression2_, *expression3_, ptr); trace.setFunction(record); @@ -625,7 +627,7 @@ class ScalarMultiplyNode : public ExpressionNode { /// Construct an execution trace for reverse AD T traceExecution(const Values& values, ExecutionTrace& trace, - ExecutionTraceStorage* ptr) const override { + char* ptr) const override { assert(reinterpret_cast(ptr) % TraceAlignment == 0); Record* record = new (ptr) Record(); ptr += upAligned(sizeof(Record)); @@ -718,13 +720,13 @@ class BinarySumNode : public ExpressionNode { /// Construct an execution trace for reverse AD T traceExecution(const Values &values, ExecutionTrace &trace, - ExecutionTraceStorage *ptr) const override { + char* ptr) const override { assert(reinterpret_cast(ptr) % TraceAlignment == 0); Record *record = new (ptr) Record(); trace.setFunction(record); - auto ptr1 = reinterpret_cast(reinterpret_cast(ptr) + upAligned(sizeof(Record))); - auto ptr2 = reinterpret_cast(reinterpret_cast(ptr1) + expression1_->traceSize()); + auto ptr1 = ptr + upAligned(sizeof(Record)); + auto ptr2 = ptr1 + expression1_->traceSize(); return expression1_->traceExecution(values, record->trace1, ptr1) + expression2_->traceExecution(values, record->trace2, ptr2); } diff --git a/tests/testExpressionFactor.cpp b/tests/testExpressionFactor.cpp index 2cc119a80..4c2ac99b1 100644 --- a/tests/testExpressionFactor.cpp +++ b/tests/testExpressionFactor.cpp @@ -188,9 +188,9 @@ TEST(ExpressionFactor, Binary) { values.insert(2, Point2(0, 0)); // Check size - internal::ExecutionTrace trace; auto traceStorage = allocAligned(binary.traceSize()); - Point2 value = binary.traceExecution(values, trace, traceStorage.get()); + internal::ExecutionTrace trace; + Point2 value = binary.traceExecution(values, trace, reinterpret_cast(traceStorage.get())); EXPECT(assert_equal(Point2(0,0),value, 1e-9)) // trace.print(); @@ -240,9 +240,9 @@ TEST(ExpressionFactor, Shallow) { // traceExecution of shallow tree typedef internal::UnaryExpression Unary; - internal::ExecutionTrace trace; auto traceStorage = allocAligned(expression.traceSize()); - Point2 value = expression.traceExecution(values, trace, traceStorage.get()); + internal::ExecutionTrace trace; + Point2 value = expression.traceExecution(values, trace, reinterpret_cast(traceStorage.get())); EXPECT(assert_equal(Point2(0,0),value, 1e-9)) // trace.print();