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

equilibrateMuscles() not called in osim.py? #133

Closed
fwillett opened this Issue Jul 6, 2018 · 7 comments

Comments

Projects
None yet
5 participants
@fwillett
Copy link

fwillett commented Jul 6, 2018

I think that equilibrateMuscles() should be called in osim.py when initializing the simulation? I noticed it wasn't being called and, when I added it, it helped some of our reaching simulations a lot by fixing bad startup behavior in the muscles.

I think it needs to be called in OsimModel's init function like this (at the end):

    state = self.model.initSystem()
    self.model.equilibrateMuscles(state)

and in reset() like this (at the beginning):

    self.state = self.model.initializeState()
    self.model.equilibrateMuscles(self.state)
@kidzik

This comment has been minimized.

Copy link
Collaborator

kidzik commented Jul 6, 2018

Thanks! Indeed that might affect the starting point, we will revise that in the upcoming release.

@kidzik kidzik added the bug label Jul 6, 2018

@kidzik kidzik added this to the Version 2.1 milestone Jul 8, 2018

@adwardlee

This comment has been minimized.

Copy link

adwardlee commented Jul 9, 2018

Is it necessary to add this function? I think it was not used in the competition last year either?

@carmichaelong

This comment has been minimized.

Copy link
Collaborator

carmichaelong commented Jul 12, 2018

It wasn't used in last year's version, but it's more important this year because the dictionary has information about the muscle states, such as fiber length and force, which may change greatly during startup due to equilibrateMuscles(). Furthermore, it brings the first step in the simulation more in line with the rest of the simulation, since equilibrateMuscles() is inherently called with each integration step.

@AdamStelmaszczyk

This comment has been minimized.

Copy link
Contributor

AdamStelmaszczyk commented Aug 18, 2018

@kidzik When this will be fixed?

@kidzik

This comment has been minimized.

Copy link
Collaborator

kidzik commented Aug 18, 2018

This is a minor issue (affecting only the first step of the simulation) which will be fixed most likely in the second round.

@AdamStelmaszczyk

This comment has been minimized.

Copy link
Contributor

AdamStelmaszczyk commented Aug 18, 2018

It's not that minor as it seems, many algorithms calculate mean and standard deviation of the observations to normalize the input. It's broken when the values in the first step are huge.

@kidzik

This comment has been minimized.

Copy link
Collaborator

kidzik commented Oct 15, 2018

We run equilibrateMuscles now in the reset function https://github.com/stanfordnmbl/osim-rl/blob/master/osim/env/osim.py#L237

@kidzik kidzik closed this Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment