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.

Sign up for GitHub

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

Jump to bottom

[WIP] Add infrastructure for state serialization #488

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

[WIP] Add infrastructure for state serialization #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3ea0cf1
Add (de)serialize-to-XML to AbstractValue and Value<T>.
sherm1 May 2, 2016
8b500bb
Merge branch 'master' of https://github.com/simbody/simbody into feat…
sherm1 May 2, 2016
72b0428
Fix merge error.
sherm1 May 2, 2016
85b9041
Fix OSX compiler error.
sherm1 May 2, 2016
ea13f91
Add operator>> to index types
chrisdembia Jun 3, 2016
3b9d20c
Add operator>> for Stage.
chrisdembia Jun 3, 2016
e904d0a
In StateImpl::fromXmlElement(), clear subsystems.
chrisdembia Jun 3, 2016
3f553e5
Throw exception if can't write to xml element.
chrisdembia Jun 3, 2016
bc063a7
Merge pull request #498 from chrisdembia/unique_index_type_istream_op…
sherm1 Jun 3, 2016
5e962bf
Merge pull request #499 from chrisdembia/stage_istream_operator
sherm1 Jun 3, 2016
ae4deaa
Merge pull request #500 from chrisdembia/state_from_xml_clear_subsystems
sherm1 Jun 3, 2016
ba07766
Fix warning with MSVC; unused variable.
chrisdembia Jun 3, 2016
272d315
Merge branch 'feature_state_serialization' of github.com:simbody/simb…
chrisdembia Jun 3, 2016
c41b785
Merge pull request #502 from chrisdembia/toXmlElement_exception
sherm1 Jun 3, 2016
59ffb36
Fix bug in ContinuousVarInfo::toXmlElement.
chrisdembia Jun 6, 2016
be5c858
Merge pull request #506 from chrisdembia/fix_State_toXml_bug
sherm1 Jun 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ to ParallelExecutor, mutex state lock.
* Started using RPATH on OSX so that users need not set `DYLD_LIBRARY_PATH` to
run `simbody-visualizer` or the example executables, regardless of where you
install Simbody.
* Index types created with the `SimTK_DEFINE_UNIQUE_INDEX_TYPE` macro and its
variants now have an `operator>>`.
* (There are more that haven't been added yet)


Expand Down
51 changes: 50 additions & 1 deletion SimTKcommon/Simulation/include/SimTKcommon/internal/MeasureImplementation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ class Measure_<T>::Variable::Implementation
}
T& updVarValue(State& s) const {
assert(discreteVarIndex.isValid());
return Value<T>::downcast(
return Value<T>::updDowncast(
this->getSubsystem().updDiscreteVariable(s, discreteVarIndex));
}

Expand Down Expand Up @@ -1289,6 +1289,43 @@ template <class T>
class Measure_Differentiate_Result {
public:
Measure_Differentiate_Result() : derivIsGood(false) {}

Xml::Element toXmlElement(const std::string& name) const {
static const int version = 1;
Xml::Element e("Measure_Differentiate_Result");
if (!name.empty()) e.setAttributeValue("name", name);
e.setAttributeValue("version", String(version));
e.appendNode(toXmlElementHelper(operand, "operand", true));
e.appendNode(toXmlElementHelper(operandDot, "operandDot", true));
e.appendNode(toXmlElementHelper(derivIsGood, "derivIsGood", true));
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't like that toXmlElementHelper has to be used here. Would be friendlier for people writing these methods if it were just toXmlElement and the mysterious third parameter were hidden somewhere below.

Copy link
Member

Choose a reason for hiding this comment

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

To clarify, you don't like that toXmlElementHelper has to be used anywhere (within toXmlElement() methods), right? Can you educate me as to why the third parameter is necessary for SFINAE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the "Helper" method should just be needed in the internal implementation. Here it is appearing in user-written code for serializing.

The issue is that we normally want classes to have toXmlElement() methods. But when serializing an existing class (std::vector, say) you can't add a new method. So you have to write a free function toXmlElement(std::vector<T>) to serialize it. What if there is both a member named MyClass::toXmlElement() and a free function toXmlElement(MyClass)? I wanted to ensure that the member gets called. By template resolution rules, that true value is a better match for a bool than an int. So I arranged things so that the member function gets called from a Helper that takes a bool while the free function gets called from a Helper that takes an int. That one gets called if there's no bool competing with it because true gets promoted to int.

All that is still necessary; I'm just thinking it doesn't have to appear in user code. Use code could call some nicely-named method, then that method could invoke the Helper which would then pick out the right member or free function. I think that should be possible ...

Copy link
Member

Choose a reason for hiding this comment

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

Thank you; that was very helpful. I also described this situation to @klshrinidhi, who may be interested in helping.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any other idea you guys have for the API would be welcome. It should be as straightforward as possible for someone to write serialization/deserialization methods for any class they want to stick into an AbstractValue in a State. I assume people will do it by copying from existing examples so it should be very predictable. The current API might be OK but it doesn't seem optimal.

Copy link
Member Author

Choose a reason for hiding this comment

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

If a specialization exists for the class, it will be used. Or the primary template is chosen, which calls the member function.

Thanks, @klshrinidhi. I think this prioritizes the free function over the member though, which is opposite of what we want.

Copy link
Contributor

Choose a reason for hiding this comment

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

It chooses the specialization over member function.
I think it makes sense because if I put the effort to specialize the function to change the serialization behavior, I would like it to be called instead of member function. Choosing the member function even when a specialization exists defeats the purpose of specializing. The person who specialized the function will be surprised to see it not take effect.
Again, I am not too familiar with this development. So you may be right.

Copy link
Member

Choose a reason for hiding this comment

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

@klshrinidhi and I just had a whiteboard session and he brought up two other points:

  1. Maybe it is desirable to prefer a free function (that is, template specialization of toXmlElement()) over a member function) if both exist. Why would one define the free function if the class already had toXmlElement()/fromXmlElement()? Perhaps the only reason would be because they want to override the behavior of the member function to achieve something new. Probable answer: the control of serializing a type should be held by that type; it would be dangerous for others to subsume that control. The resulting Xml file could not be loaded correctly by others who did not have that free function but were using the same class.
  2. Drawing a parallel to std::begin() and std::end(), @klshrinidhi was saying how you can call begin() or end() (without any namespace qualification), and if the argument is in the std namespace then std::begin()/std::end() will be called. He raised the idea that it could be nice to never have to qualify toXmlElement() (or toXmlElementHelper()) with SimTK::. This would require a <ns>::toXmlElement() in each namespace that wanted to use it, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

would anything we do here eventually change behavior if Unified Call Syntax becomes part of the standard?

Yikes! Who knows? Some of the proposals would make this easier.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moving discussion out of the code ...

return e;
}

void fromXmlElement(Xml::Element e,
const std::string& requiredName="") {
const int versionInXml = e.getRequiredAttributeValueAs<int>("version");
SimTK_ERRCHK1_ALWAYS(versionInXml == 1,
"Measure_Differentiate_Result::fromXmlElement",
"Expected Measure_Differentiate_Result version=1 but got %d.",
versionInXml);
if (!requiredName.empty()) {
const String& name = e.getElementTag();
Copy link
Member

@chrisdembia chrisdembia May 16, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Should this be e.getOptionalAttributeValue("name");? This occurs elsewhere too.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm...after looking at some more of the code, it seems there are two scenarios, one where the name is the tag name, and one where it's the value of the name attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I think that's right. That's because I made it optional to create a unique tag for serializing user objects. If the "to" method created a special tag, then the "from" method can look for it. Not sure that was a good idea, but you don't want to have to make up tags for everything (int, char, etc.). They really aren't needed because we always know what type we're expecting by the time we read it in, except for AbstractValue objects where we have the whole registration infrastructure in order to figure out what type we've got.

SimTK_ERRCHK2_ALWAYS(name==requiredName,
"Measure_Differentiate_Result::fromXmlElement",
"Expected Measure_Differentiate_Result element "
"named '%s' but got '%s'.", requiredName.c_str(), name.c_str());
}
auto nxt = e.element_begin();
fromXmlElementHelper(operand, *nxt++, "operand", true);
fromXmlElementHelper(operandDot, *nxt++, "operandDot", true);
fromXmlElementHelper(derivIsGood, *nxt++, "derivIsGood", true);
Copy link
Member Author

Choose a reason for hiding this comment

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

Not very friendly!


SimTK_ERRCHK_ALWAYS(nxt == e.element_end(),
"Measure_Differentiate_Result::fromXmlElement",
"Badly formatted Measure_Differentiate_Result element: too many "
"sub-elements.");
}

T operand; // previous value of operand
T operandDot; // previous value of derivative
bool derivIsGood; // do we think the deriv is a good one?
Expand Down Expand Up @@ -1905,6 +1942,18 @@ class Measure_Delay_Buffer {
// Return the largest capacity the buffer ever had.
int getMaxCapacity() const {return m_maxCapacity;}

Xml::Element toXmlElement(const std::string& name) const {
static const int version = 1;
Xml::Element e("Measure_Delay_Buffer");
if (!name.empty()) e.setAttributeValue("name", name);
e.setAttributeValue("version", String(version));
for (int i=0; i < size(); ++i) {
e.appendNode(toXmlElementHelper(getEntryTime(i), "time", true));
e.appendNode(toXmlElementHelper(getEntryValue(i), "value", true));
}
return e;
}

private:
// Return the i'th oldest entry
// (0 -> oldest, size-1 -> newest, size -> first free, -1 -> last free)
Expand Down
23 changes: 23 additions & 0 deletions SimTKcommon/Simulation/include/SimTKcommon/internal/Stage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,29 @@ class RealizeCheckFailed : public Base {
inline std::ostream& operator<<(std::ostream& o, Stage g)
{ o << g.getName(); return o; }

inline std::istream& operator>>(std::istream& input, Stage& g)
{
std::string levelName;
input >> levelName;
if (levelName == "Empty") g = Stage(Stage::Empty);
else if (levelName == "Topology") g = Stage(Stage::Topology);
else if (levelName == "Model") g = Stage(Stage::Model);
else if (levelName == "Instance") g = Stage(Stage::Instance);
else if (levelName == "Time") g = Stage(Stage::Time);
else if (levelName == "Position") g = Stage(Stage::Position);
else if (levelName == "Velocity") g = Stage(Stage::Velocity);
else if (levelName == "Dynamics") g = Stage(Stage::Dynamics);
else if (levelName == "Acceleration") g = Stage(Stage::Acceleration);
else if (levelName == "Report") g = Stage(Stage::Report);
else if (levelName == "Infinity") g = Stage(Stage::Infinity);
else {
SimTK_THROW4(SimTK::Exception::ErrorCheck, "valid level name",
"operator>>(std::istream&, Stage&)",
"Level name '%s' is not valid.", levelName.c_str());
}
return input;
}


} // namespace SimTK

Expand Down
14 changes: 14 additions & 0 deletions SimTKcommon/Simulation/include/SimTKcommon/internal/State.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,20 @@ inline Vector& updUErr() const; // Stage::Velocity-1 "
inline Vector& updUDotErr() const; // Stage::Acceleration-1 (not a view)
inline Vector& updMultipliers() const; // Stage::Acceleration-1 (not a view)

/** Serialize this %State to an XML Element with tag word `<State>`. If `name`
is provided the element will have an attribute `name="givenName"`. A version
number is recorded as an attribute `version="number"` where `number` is
incremented in any Simbody release that changes the %State contents. **/
inline Xml::Element toXmlElement(const std::string& name="") const;

/** Deserialize `state` from an XML Element. We expect the element to have
tag word `<State>`, and may optionally require that it has a particular value
for its `name` attribute. If the serialized version number differs from the
current one, we may attempt to convert it or will throw an exception if we don't
know how to do so reliably. **/
inline void fromXmlElement(Xml::Element element,
const std::string& requiredName="");

/** @name Advanced/Obscure/Debugging
Don't call these methods unless you know what you're doing. **/
/**@{**/
Expand Down
58 changes: 51 additions & 7 deletions SimTKcommon/Simulation/include/SimTKcommon/internal/StateImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,19 @@ class DiscreteVarInfo {
const ListOfDependents& getDependents() const {return m_dependents;}
ListOfDependents& updDependents() {return m_dependents;}

Xml::Element toXmlElement(const std::string& name) const {
static const int version = 1;
Xml::Element e("DiscreteVarInfo");
if (!name.empty()) e.setAttributeValue("name", name);
e.setAttributeValue("version", String(version));
e.appendNode(toXmlElementHelper(m_allocationStage, "allocationStage", true));
e.appendNode(toXmlElementHelper(m_invalidatedStage, "invalidatedStage", true));
e.appendNode(toXmlElementHelper(m_autoUpdateEntry, "autoUpdateEntry", true));
e.appendNode(toXmlElementHelper(*m_value, "value", true));
e.appendNode(toXmlElementHelper(m_timeLastUpdated, "timeLastUpdated", true));
return e;
}

private:
// These are fixed at construction.
Stage m_allocationStage;
Expand Down Expand Up @@ -531,6 +544,18 @@ class ContinuousVarInfo {
{ return operator=(src); }
void deepDestruct(StateImpl&) {}
const Stage& getAllocationStage() const {return allocationStage;}

Xml::Element toXmlElement(const std::string& name) const {
static const int version = 1;
Xml::Element e("ContinuousVarInfo");
if (!name.empty()) e.setAttributeValue("name", name);
e.setAttributeValue("version", String(version));
e.appendNode(toXmlElementHelper(allocationStage, "allocationStage", true));
e.appendNode(toXmlElementHelper(firstIndex, "firstIndex", true));
e.appendNode(toXmlElementHelper(initialValues, "initialValues", true));
e.appendNode(toXmlElementHelper(weights, "weights", true));
return e;
}
private:
// These are fixed at construction.
Stage allocationStage;
Expand Down Expand Up @@ -612,7 +637,7 @@ class SimTK_SimTKCOMMON_EXPORT PerSubsystemInfo {
public:
explicit PerSubsystemInfo(StateImpl& stateImpl,
const String& n="", const String& v="")
: m_stateImpl(stateImpl), name(n), version(v)
: m_stateImpl(stateImpl), m_name(n), m_version(v)
{ initialize(); }

// Everything will properly clean itself up.
Expand Down Expand Up @@ -772,12 +797,19 @@ class SimTK_SimTKCOMMON_EXPORT PerSubsystemInfo {
SimTK_FORCE_INLINE StageVersion getStageVersion(Stage g) const
{ return stageVersions[g]; }

Xml::Element toXmlElement(const std::string& name) const;
static void fromXmlElement(PerSubsystemInfo& subsys,Xml::Element e,
const std::string& requiredName) {
SimTK_ASSERT_ALWAYS(!"not implemented",
"PerSubsystemInfo::fromXmlElement()");
}

private:
friend class StateImpl;
ReferencePtr<StateImpl> m_stateImpl; // container of this subsystem

String name;
String version;
String m_name;
String m_version;

// DEFINITIONS //

Expand Down Expand Up @@ -1007,8 +1039,8 @@ class SimTK_SimTKCOMMON_EXPORT StateImpl {

void initializeSubsystem
(SubsystemIndex i, const String& name, const String& version) {
updSubsystem(i).name = name;
updSubsystem(i).version = version;
updSubsystem(i).m_name = name;
updSubsystem(i).m_version = version;
}

SubsystemIndex addSubsystem(const String& name, const String& version) {
Expand All @@ -1020,10 +1052,10 @@ class SimTK_SimTKCOMMON_EXPORT StateImpl {
int getNumSubsystems() const {return (int)subsystems.size();}

const String& getSubsystemName(SubsystemIndex subsys) const {
return subsystems[subsys].name;
return subsystems[subsys].m_name;
}
const String& getSubsystemVersion(SubsystemIndex subsys) const {
return subsystems[subsys].version;
return subsystems[subsys].m_version;
}

// Make sure the stage is no higher than g-1 for *any* subsystem and
Expand Down Expand Up @@ -2100,6 +2132,9 @@ class SimTK_SimTKCOMMON_EXPORT StateImpl {
String toString() const;
String cacheToString() const;

Xml::Element toXmlElement(const std::string& name) const;
void fromXmlElement(Xml::Element e, const std::string& requiredName);

private:
// This is the guts of copy construction and copy assignment which have to
// be done carefully to manage what gets copied and whether the resulting
Expand Down Expand Up @@ -2856,6 +2891,15 @@ inline Vector& State::updMultipliers() const {
return getImpl().updMultipliers();
}

inline Xml::Element State::
toXmlElement(const std::string& name) const
{ return getImpl().toXmlElement(name); }

inline void State::
fromXmlElement(Xml::Element element,
const std::string& requiredName)
{ return updImpl().fromXmlElement(element, requiredName); }

inline void State::
setSystemTopologyStageVersion(StageVersion topoVersion)
{ return updImpl().setSystemTopologyStageVersion(topoVersion); }
Expand Down
Loading