I remember this from the workshop! It seems that you've introduced tab characters for indenting. Could you replace those tabs with 4 spaces? Most editors have a setting (e.g., retab in vim, untabify in visual studio).

Thanks, Daniel! Looks like there may be a merge conflict, or maybe you just need to merge master into your branch. The automated build failed with the error below. the ContactGeometry::Brick type was added a few months ago (to ContactGeometry.h) but is somehow undefined in your PR.

/home/travis/build/simbody/simbody/SimTKmath/Geometry/include/simmath/internal/ContactTracker.h: In constructor ‘SimTK::ContactTracker::HalfSpaceBrick::HalfSpaceBrick()’:
/home/travis/build/simbody/simbody/SimTKmath/Geometry/include/simmath/internal/ContactTracker.h:234:37: error: ‘SimTK::ContactGeometry::Brick’ has not been declared

BTW, if you look on the conversation page for a PR you can see the Travis CI build status -- it gets built when you first submit the PR and then every time you add a commit to it.

I'm a noob regarding github and travis CI. How do I merge my branch into the master?
The thing is that the codeworks fine in the latest Simbody release (v3.4.1). Up untill now I only coded offline, so maybe the best would be to "merge for the kill".

Ok! Apparently the integration went smoothly this time:

"The pull request can be automatically merged by project collaborators"

:)

Let me know what other functions are required to integrate superellipsoids
with the contact force models. I
t would be nice to make a TODO list for this next step of the
superellipsoid integration.

Cheers!

Daniel Simões Lopes, PhD
http://web.ist.utl.pt/daniel.s.lopes/
Visualization and Intelligent MultiModal Interfaces Group
INESC-ID / Tagus Park
Tel.: +351 21 040 0765 - Ext.: 5265

On Fri, Nov 28, 2014 at 10:47 PM, Christopher Dembia <
[email protected]> wrote:

Ah sorry Daniel!

—
Reply to this email directly or view it on GitHub
#271 (comment).

I'm a noob regarding github and travis CI. How do I merge my branch into the master?

The general GitHub ethic is that no one merges their own PR into master. Instead, the idea is that other team members review the PR, and then after changes, someone (other than the author) does the merge. Occasionally this rule gets broken for minor changes, but that's the basic idea. I'm working on filling in the CONTRIBUTING.md file that will explain some of that.

Unfortunately this creates some delays if other team members are swamped (as is the case at the moment). We are about to put out the Simbody 3.5 release (from the master branch) and wouldn't want to introduce substantial changes to it at the last minute anyway.

Hey @danielsimoeslopes. We're working on improving our contributing guidelines to help make the contributing process smoother for contributors. We're working on some guidelines here); it'd be great if you could read through them. I'd also like to provide some helpful tips about GitHub. My apologies if you already understood some of this.

1. Once you made your pull request (PR), it seems that your workflow was to check travis/appveyor test results, and make the necessary modifications to files via the GitHub website. The travis/appveyor tests are useful for presenting test results to users other than the author (e.g., @sherm1), and for testing on platforms other than the one the author develops on. Even after you've created a pull request, it is often easier to continue developing offlinen/locally on your own machine. When tests pass locally, you can then push your commits to GitHub. As you update your own master branch by pushing from your local repository, this PR will be updated with those commits. By developing locally, you can avoid having to wait for travis/appveyor to finish building.
2. When working on a new feature, it is convention to create a new feature branch and add the feature on that branch instead of on the master branch. I see that you've committed your work to the master branch. In the future, it'd be great if you could create a new branch (e.g., superellipsoid), and commit to that branch. Don't worry about this for this time around, though.

Thanks you for the feedback!
Next time, I will follow your guidelines more more thoroughly.

Hi, Daniel. We just finished creating a set of contributor's instructions specifically for Simbody. Since you happen to have a nice big contribution pending, it would be great if you could take a look at it and give us some feedback on it. Is it helpful? Is it clear? What else would be helpful? The instructions are in the CONTRIBUTING.md file at the top level of the Simbody repo.

Thanks and regards,
Sherm

Hi Sherman,

Just read the CONTRIBUTING.md
https://github.com/simbody/simbody/blob/master/CONTRIBUTING.md file and
it looks great! It really helps newcomers on how to handle with GitHub.
After reading the file, I was clarified on how to correctly make a PR among
other coding things.

I think it would be nice to briefly mention travis/appveyor as it gives the
developer a nice interface to see build information. Maybe something
similar to what Chirstopher Dembia wrote in previous e-mail:

"Once you made your pull request (PR), it seems that your workflow was to
check travis/appveyor test results, and make the necessary modifications to
files via the GitHub website. The travis/appveyor tests are useful for
presenting test results to users other than the author (e.g., @sherm1
https://github.com/sherm1), and for testing on platforms other than the
one the author develops on."

Best regards,
DSL

On Wed, Dec 10, 2014 at 5:12 AM, Michael Sherman [email protected]
wrote:

Hi, Daniel. We just finished creating a set of contributor's instructions
specifically for Simbody. Since you happen to have a nice big contribution
pending, it would be great if you could take a look at it and give us some
feedback on it. Is it helpful? Is it clear? What else would be helpful? The
instructions are in the CONTRIBUTING.md
https://github.com/simbody/simbody/blob/master/CONTRIBUTING.md file at
the top level of the Simbody repo.

Thanks and regards,
Sherm

—
Reply to this email directly or view it on GitHub
#271 (comment).

Hi, Daniel. We just got the Simbody 3.5 release out today so I took a look at the superellipsoid code you submitted. Probably the biggest issue right now is that the code needs to be finished before it is ready to review. The current state appears to be that the code was copied from the existing Ellipsoid classes, and then was changed in a few places to implement the SuperEllipsoid classes. However, much of the code hasn't changed. For example, large blocks of comments remain that are actually describing ellipsoids, even though they are in the superellipsoid code. Also, there are several methods in the SuperEllipsoid code, for example findUnitNormalAtPoint(), whose implementation appears to be the ellipsoid one. You can see from the previous link that the doxygen comment refers to ellipsoids, and the implementation here doesn't seem to have changed. Even for some methods that have the superellipsoid implementation, the comments in the header files still describe ellipsoids.

Probably the best thing to do if you still want to do this is to go through the code line by line and method by method, treating it as kind of a things-to-do-list to complete the project. Also you will need to add a testSuperEllipsoid() subtest to the TestContactGeometry.cpp regression to make sure all the methods are working now and that someone doesn't break them in the future.

If you want to bite off a smaller piece first, you might consider first submitting just the DecorativeGeometry and Visualizer changes in their own PR. My impression is that those could be done separately from the contact code -- is that right? That would provide a chance for you to go through the whole process once and get some code into the repo.

Please let us know your thoughts and plans. It would be great to get superellipsoids as a contact type but it looks like quite a lot of work to complete the project. Unfortunately we don't have anyone here with time available at the moment, so it is up to you -- we need more volunteers!

Regards,
Sherm

Hi Sherman!

Well, my plan is to implement superellipsoids in Simbody, so count me in
for this :)
I will go through the code and list what can be done and what would belong
to a TODO list.

Best regards,
Daniel

On Fri, Dec 19, 2014 at 12:09 AM, Michael Sherman [email protected]
wrote:

Hi, Daniel. We just got the Simbody 3.5 release out today so I took a look
at the superellipsoid code you submitted. Probably the biggest issue right
now is that the code needs to be finished before it is ready to review. The
current state appears to be that the code was copied from the existing
Ellipsoid classes, and then was changed in a few places to implement the
SuperEllipsoid classes. However, much of the code hasn't changed. For
example, large blocks of comments remain that are actually describing
ellipsoids, even though they are in the superellipsoid code. Also, there
are several methods in the SuperEllipsoid code, for example
findUnitNormalAtPoint()
https://github.com/simbody/simbody/pull/271/files#diff-8e5a5093f34230aa59cf6d7cbc8603bbR1125,
whose implementation appears to be the ellipsoid one. You can see from the
previous link that the doxygen comment refers to ellipsoids, and the
implementation here doesn't seem to have changed. Even for some methods
that have the superellipsoid implementation, the comments in the header
files still describe ellipsoids.

Probably the best thing to do if you still want to do this is to go
through the code line by line and method by method, treating it as kind of
a things-to-do-list to complete the project. Also you will need to add a
testSuperEllipsoid() subtest to the TestContactGeometry.cpp
https://github.com/simbody/simbody/blob/master/SimTKmath/tests/TestContactGeometry.cpp
regression to make sure all the methods are working now and that someone
doesn't break them in the future.

If you want to bite off a smaller piece first, you might consider first
submitting just the DecorativeGeometry and Visualizer changes in their own
PR. My impression is that those could be done separately from the contact
code -- is that right? That would provide a chance for you to go through
the whole process once and get some code into the repo.

Please let us know your thoughts and plans. It would be great to get
superellipsoids as a contact type but it looks like quite a lot of work to
complete the project. Unfortunately we don't have anyone here with time
available at the moment, so it is up to you -- we need more volunteers!

Regards,
Sherm

—
Reply to this email directly or view it on GitHub
#271 (comment).

Well, my plan is to implement superellipsoids in Simbody, so count me in
for this

Great! Glad to hear that, Daniel.