-
Notifications
You must be signed in to change notification settings - Fork 324
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
[WIP] Exponential Contact #3310
base: main
Are you sure you want to change the base?
Conversation
OpenSim::ExponentialSpringForce compiles and links. No default constructor. Properties are: 1. SimTK::Transform - contact_plane_transform 2. PhysicalFrame - body 3. SimTK::Vec3 - body_station For now, the default parameters will be used. Next step is to create an example to test simulation, serialization, and reporting. Once these aspects look good, I'll add ExponentialSpringParameters so that a spring instance can be customized.
Just has HuntCrossleyForce at the moment. Just getting the bones of the class together.
Started adding the Visualizer.
I'm getting run-time exceptions. It looks like I cannot run the SimTK::Visualizer in OpenSim by accessing it in a raw way. It looks like I should be using OpenSim::ModelVisualizer, which creates its own SimTK::Visualizer and SimTK::Visualizer::InputSilo.
Adding SimTK decorations is not working, so I avoid these though the code is still there.
I also added a default constructor and `extendConnectToModel()`. The springs work in OpenSim!
Things are running! I can also run both the ExpSpr and HuntCross block at the same time. Basic computational performance is printed out at the end.
- Check for nullptr before disowning and deleting model. - Changed SpinSlide into just Spin. - Tweaked initial conditions.
I changed the class name from OpenSim::ExponentialSpringForce, which might have been confused with SimTK::ExponentialSpringForce, to OpenSim::ExponentialContact. I also added the topology-stage parameters. On the SimTK side, they are managed by SimTK::ExponentialSpringParameters. I tried not to reproduce functionality on the OpenSim side, but to make use of SimTK::ExponentialSpringParameters as much as possible. OpenSim::ExponentialContact has its own "Parameters" class that encapsulates a SimTK::ExponentialSpringParameters instance. The main purpose of OpenSim::Parameters is to establish and manage all of the OpenSim Properties that correspond to the member variables in SimTK::ExponentialSpringParameters.
Bug Fix- ExponentialContact::getParameters() can be called before the Model is initialized. The resolution of the call should therefore go through the ExponentialContact::Parameters instance and not the pointer (_spr) to theunderlying SimTK::ExponentialSpringForce instance. Minor tweaks included the addition of a "const" qualifier and some comments.
testExponentialSpring.cpp is now working with default and non-default contact parameters. Testing is the exhaustive, but some basic scenarios work. I also added some comments and made some small polishing tweaks to the code.
This name change made the name shorter and will help distinguish the OpenSim classess (ExponentialContact) from the Simbody classes (ExponentialSpringForce).
- Took out a custom visualization class that was not currently being used. - Now deleting the objects that are allocated from the heap. - Corrected some of the comments.
Also needed to register class ExponentialContact::Parameters.
Bug Fix: "viscosity" was misspelled in a number property related methods. I added a method that tests if the OpenSim Properties and SimTK Parameters are consistent with each other.
ExponentialSpringForce -> ExponentialContact, and the abbreviation at the end of variable names when from "ES" to "EC". I also re-organized some of the model initialization so that static model testing could precede the simulation. Basically, a model needs to be completely put together before meaningful tests can be conducted, AND the state needs to be initialized (and the SimTK::System needs to be realized at Stage::Topology) just prior to integration.
Now deleting contact geometry associated with the Hunt-Crossley springs. Changed corner[n] from a local variable to a member variable so that it only needs to be initialized once.
I'm not yet finished adding tests, but the code is running.
1) SpinSlide 2) SpinTop
… Variables. In the code, search on "F. C. Anderson" to locate the substantive changes. These changes are accompanied by detailed comments. 1. Added data members ('subsystem' and 'allocate') to struct DiscreteVariableInfo. 2. Added the variable 'bool allocate' to the argument list of addDiscreteVariable(), which allows prevention of double allocation of a Discrete Variable in Component::extendRealizeTopology(). 3. Added a new method-- updDiscreteVariableIndex(). This method allows a derived (concrete) Component class to initialize the indices and specify the Subsystem for Discrete Variables that the derived Component owns. 4. In getDiscreteVariableValue() and setDiscreteVariableValue(), it is no longer assumed that the Subsystem to be used is the SimTK::DefaultSystemSubsystem. Instead, struct DiscreteVariableInfo is consulted. 5. Finally, in extendRealizeTopology() the 'allocate' data member of the DiscreteVariableInfo struct is consulted. If 'allocate' is 'true', the Discrete Variable is allocated normally. If 'false', allocation is left to the derived (concrete) class.
…pensim-core into exponential_springs
…ble. I also - refined a few documentation comments. - added getSubsystem() as a more general method for getting the non-default subsystem.
The code is not working in this current commit. Continuing from the current modifications, I am going to experiment with several approaches to support serializing discrete Variaibles. Rather than making changes in the current code, I'm going to branch. When I find an approach that works, I will merge back.
These methods were a work in progress [wip] and were never actually functioning. They will only be needed if the decision is to go forward with serializing/deserializing discrete states. If so, then it's probably best to start from a clean slate on these anyway. This commit is also accompanied by a few minor typo corrections.
I should add some clarifying notes... In commit 437feb7, I added a few accessor (get/set) methods to class Component to handle Discrete Variables (DVs) that are not type double. These accessors address point 3 in my previous comment. The original accessor methods are:
The added accessor methods are:
In Simbody, DVs are handled using
returns an
The added accessor methods allow OpenSim to interface with DVs that are not type double via the Component API. Note that because the signatures of the original accessor methods haven't changed, no changes are needed elsewhere in OpenSim. That is, other classes can keep interacting with Components as usual, provided the DVs are type double. |
@fcanderson, in Moco, we use a class we created called DiscreteController, which allows us to store the values of the controls in the model as discrete variables (so we can carry them around with the state). Something like this for your class could make sense here for serializing/deserializing. I also like @aseth1's idea to update |
@nickbianco, thanks for sending the .h file for the DiscreteController. It's nice that the interface is so clean. I look forward to brainstorming with you and identifying the most promising/productive way forward. Thanks again for the time you are spending on this! |
@nickbianco, just a quick update. I've implemented the capability to access (get and set) a Discrete Variable by specifying its absolute component path-- a step toward being able to serialize/deserialize all discrete variables in a model. I will clean up the code and enhance the testing this weekend, and then commit my additions early next week. That's the plan anyway. Also, just wanted to note that the reason for the check failures on GitHub is that some changes I made to the Simbody contact model (SimTK::ExponentialSpringForce) haven't been merged into the Simbody master yet. Before requesting a merge from Michael Sherman, I figured I would wait until everything is functioning as desired on the OpenSim side and I'm relatively confident that no more tweaks to SimTK::ExponentialSpringForce are needed. If you want to run my latest code, you'll have to check out and build my Simbody branch: https://github.com/fcanderson/simbody/tree/master |
Component methods getDiscreteVariableAbstractValue() and updDiscreteVariableAbstractValue() will now access the value of a discrete variable based on a specified Model hierarchy path. I added a utility method, Component::resolveDiscreteVariableNameAndOwner(), to support this functionality. These additions do not require any adjustments to existing OpenSim code. They are intended to support the ability to serialize and deserialize discrete variables so that a complete SimTK::State can be captured. I have included some testing code in testExponentialContact.cpp that exercise this new functionality.
@nickbianco The cleanup and testing went faster than expected. That doesn't happen very often for me. I have committed my latest round of changes. A discrete variable can now be accessed via the OpenSim::Component API by specifying the path of the discrete variable. That is, as necessary, the getter and setter code traverses the Component tree to locate the discrete variable. I will next take a look at the OpenSim Storage, Table, and StateTrajectory classes to see how discrete variables might be efficiently and reproducibly serialized. |
OpenSim/Common/Component.h
Outdated
* | ||
* To obtain the type-specific value of a discrete variable, perform | ||
* a cast using the template methods provided in class SimTK::Value<T>. | ||
* When the type is unknow, it can be querried using the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of typos here: unknow
--> unknown
and querried
-- > queried
.
OpenSim/Common/Component.h
Outdated
* | ||
* To obtain the type-specific value of a discrete variable, perform | ||
* a cast using the template methods provided in class SimTK::Value<T>. | ||
* When the type is unknow, it can be querried using the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same typos here.
@fcanderson the new changes are looking good! I did a quick pass over everything (and left some quick comments for some typos), but I'll leave a full review once everything is finalized. For serialization and deserialization, you'll want to look at |
@nickbianco thanks for the review. I'll get those typos fixed. I have been looking at What looks best to me is following through on the proposed What are your thoughts on proceeding with this approach? I would essentially need to implement something like the following methods:
I would also need to write an OStatesDoc class, which would encapsulate the XML DOM object and handle things like setting/getting the appropriate XML attributes as well as the data held in the XML child nodes (i.e., the individual trajectories of the SimTK states). For the XML Document, I'm thinking something like this...
|
The typos were in Doxygen comments.
The states are output using an OpenSim::Storage file.
@fcanderson, I think this could be a reasonable approach! At first, I was worrying about compatibilty with other classes/functions that need For larger trajectories, do you find this file structure to be unwieldy at all? How about the file size? |
@nickbianco Excellent point about compatibility with other classes. But great solution! It is nice that we will be able to go back and forth between the different formats via a StatesTrajectory. Keep in mind, however, that while a StatesTrajectory object will have all states (Continuous and Discrete), there will likely be issues encountered with Storage objects and TimeSeriesTable objects containing DiscreteVariables. For example, TimeSeriesTable looks like it assumes that all data are of the same type, which is in large part the motivation for creating an OStatesDoc class where type can vary. I am not too worried about file size. The tags will represent a very small portion of the characters that appear in a file. The possible exception being the tags associated with data types like Vec3. The most common data type will double, and I envision not having a tag for doubles. In addition, the tags for types like Vec3s are not necessary. They are just there to make it easier to read should someone open the file in an editor. Removing type tags is a call that can be made if the file sizes become unwieldy. Most of the file size will come from the number of decimal places chosen for the data. For reproducing the states, it seems we should write numbers out in full precision. I will proceed with this plan! It strikes me as the best way forward. Everything seems to line up. |
ps - I am going to branch my current OpenSim fork to add in the OStatesDoc functionality, just to keep things a bit more compartmentalized. |
@fcanderson I just realized that
Have you tried creating a |
@nickbianco Ooooo. Ok. Since there were still "TODO" items at the end of StatesTrajectory.h I figured that the .ostates format hadn't been implemented yet. I took the statements to which you directed me to be a roadmap for future implementation, not something that had already been implemented. I haven't spent any time programming for the last two days, so no time lost. I will try what you suggest to see what's there before I implement anything. Thanks for the msg! |
@nickbianco It looks like the I am going to proceed with the |
@fcanderson, I clearly read through the header too quickly and didn't notice the TODO above the |
@nickbianco I will definitely consult class Object as I implement the OStatesDoc class. My inclination is to keep the serialization formats for an Object and OStatesDoc separate, although there might be considerable borrowing from what Object does. Since the .ostates files will be somewhat large, speed will be a priority. Best to keep the OStatesDoc class as lightweight as possible I think. In addition, it might be wise to allow some flexibility for the format to change and optimize as we become more familiar with the what we want from a .ostates file. |
A vector of Simbody::State objects is saved during the simulation at a time interval of 0.001 sec. There is, however, no way to serialize the Discrete Variables. Only the Continuous States can be serialized via a TimeSeriesTable.
@jenhicks @aymanhab @carmichaelong @nickbianco @aseth1 @adamkewley @tkuchida
Apologies in advance; this is a LONG writeup. I thought it best, though, to take the time to get the big-picture stuff written down. The info builds toward some questions that I formulate toward the end.
This PR contains a first draft of class
ExponentialContact
. ClassExponentialContact
uses an exponential spring as a means of modeling contact of a specified point on aBody
with a contact plane that is fixed toGround
.The underlying mechanics are implemented natively in Simbody. In Simbody, the code resides in 5 files:
ExponentialSpringForce.h
and.cpp
,ExponentialSpringForceImpl.cpp
, andExponentialSpringParameters.h
and.cpp
. This code is accompanied in the Simbody build system by a utility for visualizing and evaluating performance ([Test_Adhoc] ExperimentalSpringsComparison.cpp
), an extensive unit test ([Test_Regr] testExponentialSpringForce.cpp
), and Doxygen-compliant comments in.h
files. The code was reviewed by Michael Sherman.OpenSim::ExponentialContact
is essentially a wrapper class ofSimTK::ExponentialSpringForce
. The name change between OpenSim and SimTK was made to better assist potential users in identifying the purpose of the class (e.g., it's a contact class with friction, not a simple spring actuator) and to reduce confusion between the two classes.To write class
ExponentialContact
, I basically mimicked classOpenSim::HuntCrossleyForce
.IMPORTANT:
ExponentialContact
relies on Simbody d685ed2 (PR #746) or later.PR #746 delivered some performance and API enhancements to class
ExponentialSpringForce
. It was merged into Simbody on Oct. 26, 2022. To compile this PR, you'll need to update to d685ed2 or later.This PR is tagged as a work-in-progress [WIP] because some aspects of
ExponentialContact
don't fully function as would be expected for anOpenSim::Component
.In the remainder of this PR, I'll go over an Inventory of Assets, What's Working, What's Not Working, and Questions / Issues.
Inventory of Assets
Class
ExponentialContact
Class
ExponentialContact
is the wrapper class forSimTK::ExponentialSpringForce
. It is implemented in two files in OpenSim-Core:ExponentialContact.h
andExponentialContact.cpp
. It encapsulates subclassExponentialContact::Parameters
to handle non-default parameter choices for quantities such as stiffness, viscosity, coefficients of friction, etc., and interface with the underlyingSimTK::ExponentialSpringParameters
class. Fairly detailed Doxygen-compliant comments are in place in fileExponentialContact.h
. An introduction explains how the class works, and each method is accompanied by a description and Doxygen-style argument list using the @param keyword.Test Utility (
testExponentialContact.exe
)A command-line utility is available (see
testExponentialContact.cpp
) to evaluate the performance of classExponentialContact
relative to classHuntCrossleyForce
for a bouncing 10-kg, 6-dof block. The utility takes a set of command arguments that can be used to select a) between pre-set initial conditions, b) which contact model is used (both at the same time is possible), c) whether or not damping is present, d) if a ramping external force is applied, and e) whether or not visuals are shown. Upon completion, a summary of modeling choices, integrator settings, and cpu times are printed:A routine (
void testExponentialContact()
) was also added totestForces.cpp
. This routine does very much the same thing as rountinestestElasticFouncation()
andtestHuntCrossleyForce()
.What's Working
Some basics of class
ExponentialContact
are working in OpenSim. Here's a partial list that hits the high points:ExponentialContact
instance can be assembled and used within an OpenSim model by specifying the name of the body on which the instance acts, along with a few other required constructor arguments.ExponentialContact
runs anywhere from1x
to10x
faster thanHuntCrossleyForce
, depending on the circumstances (e.g., sliding or static).ExponentialContact
instances are serialized and deserialized (as XML) viaProperties
.ExponentialContact
instances may be obtained via aForceReporter
. Side Note - I don't think I filled out the storage with the expected entries. I did not, for example, convert the applied forces into generalized body force.What's Not Working
Although I have done a fair amount of reading, I don't yet have a clear picture of the essential functionality required by an
OpenSim::Component
. I know enough, however, to realize that classExponentialContact
falls short of satisfying important functionality in the following categories.States.
The underlying Simbody Subsystem,
ExponentialSpringForce
, has 4 state variables.There are 2 Discrete States: 1) Static Coefficient of Friction (
MUS
) [Real
] and 2) Kinetic Coefficient of Friction (MUK
) [Real
]. These 2 states, as discrete variables, can be changed discontinuously during a simulation without invalidating the System Topology. They are intended to enable the user to simulate a slippery spot on the floor, for example.There are 2 Auto Update Discrete States: 1) Elastic Anchor Point (
p₀
) [Vec3
] and 2) Sliding (K
) [Real
]. From an initial value, the values of these 2 states evolve over the course of a simulation. Because they cannot be computed uniquely from the other states of the System, they cannot be treated simply as data cache entries.Currently, none of these states are exported to OpenSim. They are hidden beneath the covers.
Data Cache Entries
Similarly, the private implementation of
ExponentialSpringForce
(i.e.,ExponentialSpringForceImpl
) possesses an extensive data cache, which is listed below.Data cache indices are acquired in
ExponentialSpringForceImpl
, but not for each individual quantity. Rather, just one index for each struct at the separate realization levels listed above (Pos, Vel, and Dyn).Most of the above quantities are accessible via standard accessor methods in class
ExponentialSpringForce
on the Simbody side. A subset of these are also currently available in OpenSim via an identical pass-through API.However, just as for the states, OpenSim does not know that these quantities are data cache entries. Therefore, in the present version of ExponentialContact, it is not possible to access these entries via the Data Cache API provided by class Component.
Inputs, Outputs, & Sockets
Class
ExponentialContact
does not currently define any inputs, outputs, or sockets. So, the class is not plug-n-play. It would be cool to get that working though!Questions / Issues
MUS
andMUK
(see above) in OpenSim. As they have already been allocated as part of the Simbody System, however, it seems that I will need to do something other than useComponent::addDiscreteVariable()
. Is there a way to generate the proper map entries on the OpenSim side without allocating a new discrete state variable? Can I just manually add the proper map entries? If so, is the order important?p₀
andSliding
in OpenSim? They are members of the underlying Simbody State. If there are times when OpenSim serializes or deserializes the OpenSim State and then pushes that State to the Simbody State, there might be issues.Component
API for accessing data cache information? Since the data cache entries have already been allocated on the Simbody side in the private implementation, I would need to add fake map entries and redirect queries to the correct accessor method.Thanks!
Thank you to those of you who managed to make your way through the post. I realize you are all quite busy with many things.
I look forward to hearing back from any of you in a position to lend some expertise. A little guidance, particularly when it comes to class OpenSim::Component, will go a long way.
-Clay
This change is