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

Implementation of first version #1

Merged
merged 152 commits into from
May 16, 2019
Merged

Implementation of first version #1

merged 152 commits into from
May 16, 2019

Conversation

S-Dafarra
Copy link
Owner

@S-Dafarra S-Dafarra commented Jan 28, 2019

This PR implements the first version of the sDiff tool. This tool mimics automatic differentiation for expressions where known blocks are involved.

The code is built around Eigen matrices, uses templates extensively and is header only.

A proper readme is missing.

I am open to suggestions for a fancier name.

FYI
@DanielePucci, @traversaro, @GiulioRomualdi, @diegoferigo

TODO

  • README
  • Change name
  • CI

Now it inherits from Evaluable.
This allows to output an expression as derivative.
…orsEvaluables.

Missing row and element evaluables.
Commented the derivative parts to test them one by one.
It checks whether an evaluable is dependent on a variable.
@traversaro
Copy link

Cool. What's the plan? It will be a dependency of iDynTree? Or of the dynamic planner code?

@traversaro
Copy link

Do you want to move the repo to robotology-playground ?

@S-Dafarra
Copy link
Owner Author

The plan would be to test it first inside the planner, also to check whether it is already enough for the use cases I need. Afterwards I was thinking to move it in one of our organizations.
One idea then could be to use it to simplify the definition of custom optimal control problems.

Copy link

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

Minor cmake comments.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
cmake/sDiffConfig.cmake.in Outdated Show resolved Hide resolved
@traversaro
Copy link

I am open to suggestions for a fancier name.

In view of future move to robotology, I suggest to use a repo name with just lowercase characters and eventually a - to separate words.

Copy link

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

Comments

@traversaro
Copy link

I would set up the CI with at least Visual Studio as soon as possible, such template heavy code is typically a good way to encover bugs in compilers. : )

@S-Dafarra
Copy link
Owner Author

I would set up the CI with at least Visual Studio as soon as possible, such template heavy code is typically a good way to encover bugs in compilers. : )

Definitely agree. Do you have any entry point for this?

Allowing to specify the name of generics, helpers and commons.
…bit the time to compute the squeeze evaluable.

Improved definitionof the operator == and !=.
This remove the need of setting to false all the entries of the register in case of new values.
The concept of Registrar is introduced, whose aim is to avoid having to go through all the expression tree to know if the expression is new. Indeed, only few evaluables can change their values.
In particular, these are variables and mutables. The latter are similar to constants with the difference that they have defined the operator equal. All the methods isNew and isDependent from
have been substituted by the method addDependencies where the actual dependencies are specified.
Having raw pointers allows to store (this) as dependency. In turn this allowed to define the Assignable object, where the operator = is defined, while avoiding to override the getDependencies method.
In turn, the static check for the = operator is easier. This seemed to be causing issues in VCC and clang.
@S-Dafarra
Copy link
Owner Author

This PR has definitively gone out of control. Sorry for the spam. Merging as is, facing the remaining problems in other issues.

@S-Dafarra S-Dafarra merged commit 3256134 into master May 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants