Does Component need a constructor that takes a filename? Does Component need to call finalizeFromProperties() in its constructors?

[chrisdembia]

It's possible that constructing Model invokes two calls of finalizeFromProperties().

[jenhicks]

We should remove the constructor that takes a filename.

[aymanhab]

I'm not sure which class we're referring to here but constructors from filenames are used all over the place from Models to Tools and in almost every single example piece of code we publish!

[chrisdembia]

We're talking about removing Component::Component(std::string). It's an abstract class, so it makes less sense to have this constructor. At least it should be protected (though since it's abstract, maybe it's already implied as protected).

[chrisdembia]

The issue is that having this constructor causes too many invocations of finalizeFromProperties()

[aseth1]

I think there are at least two issues being mangled here. First is whether or not Component should have a constructor which takes filename which it uses to construct as an Object that deserializes from that file. I am fine if we decide to remove it but I don't think it is causing any problems, and if you derive a concrete Component and you want that deserializing constructor you have to explicitly call Object(filename) from your constructor instead of calling the Component(filename), which may be weird for some.

Second, finalizeFromProperties() was being called excessively because of a bug that was causing the isObjectUpToDateWithProperties() to be false and triggering subsequent finalizes to be invoked unnecessarily (see #1100) . That issue has been addressed (#1120).

Third is whether or not Model constructors should have the additional power of finalizing itself upon construction (which invokes the virtual extendFinalizeFromProperties() on Model). No other Component can do this. This was done mostly for convenience since a Model is dead (no traversable tree of subcomponents) without a top-level finalizeFromProperties(). So immediately after constructing a Model you would have to call finalizeFromProperties() always. That seemed to burdensome for me, but in hindsight I think it is the correct thing to do, because now if you copy construct a Model you do have to finalizeFromProperties yourself and it exposes an inconsistency (see #1130) that is bothersome.

Model is doing double duty as both top-level (root and owner of the System) and a bag of stuff (albeit a complex bag). Some ideas between @tkuchida and @klshrinidhi (sorry I cannot remember who brought it up) is to introduce a permanent built-in top-level (Environment, Study, World, etc...) to be the root (set itself on Model(s)) and call whatever else we liked on a Model to have it ready for connecting/simulating, e.g. World::loadModel(fileName). I suggest we defer doing this to post 4.0 (unless it solves other issues as well) and remove finalizeFromProperties() from being invoked inside Model constructors. This will eliminate the inconsistency and side-effect of calling a virtual method in the constructor and leave us open to having a top-level later, but it will force us now to update all our tests and examples.

[aymanhab]

@aseth1 Thanks for the nice summary. I agree on waiting for major changes (e.g. World/Environment) till post 4.0 when we have full hierarchical models, so the distinctions are justified. Can you write what the code snippet for model initialization from file would look like under your proposal? As of now it looks like (about a couple hindered times in our code base): Model m = Model('file.osim'); State& s = m.initSystem();

Also if this changes what are the allowed operation post construction and before initSystem if any?

[jenhicks]

Proposal is to remove the step of building the component tree from finalizeFromProperties and update the method for traversing through the component tree.