Alneos / vega

Finite element format converter
GNU General Public License v2.0
23 stars 9 forks source link

Using ordered maps in the Container class #14

Closed ThomasAbballe closed 5 years ago

ThomasAbballe commented 7 years ago

Hi all,

The Model class rests heavily on the Container class, which regroups our "stack" of analyses, constraints and so on. It is, basically, an _unorderedmap class with a few very useful functions.

From a physical point of view, there is no reason to sort or order the various parts of the models. A force is a force, whatever its order in our map. It may have been simpler and quicker to implement, and yields a quicker translation. I am not expert enough on C++11 to judge on that.

From a user and developer point of view, however, it is an handicap. As there name says, unordered map don't offer any order when looping on them. As we use loops everywhere, especially in the writer, a few problems arise:

Most users will never see this, as they will translate their model ONE time on ONE computer. And I'm pretty sure their viewers will then display the result more or less in the same way. But it's cumbersome for testing (especially for non-regression tests!) and for developing.

For these reasons, I want to get rid of this maps and replace them with ordered maps (or an other suitable C++ container). It's not urgent. And, as of today, it's too technical an issue for me, i can't seem to make sense of all theses hash_something_tables. What are your thoughts on the subject ?

ldallolio commented 7 years ago

Hi Thomas,

I am sorry I cannot look in detail at this moment, could you please check that a Container is really, as you say, unordered ? I agree with you that it should not be. If I recall correctly, a Container has (not is) a lightweight ordered map "by_id" that can be used by the writer to write things in a stable order:

    template<class T> class Container final {
        std::map<int, std::shared_ptr<T>> by_id;
        std::unordered_map< typename T::Type, std::map<int, std::shared_ptr<T>>,
        std::hash<int>> by_original_ids_by_type;

If I understand what you need, thought, you would like to keep "insertion order", that is different from using map, that is "key ordered". In this case you should keep track of objects using both a list and a map, as we do for nodes and elements.

Also, unordered maps are much faster than ordered ones, this is the reason we use them ;-) We wanted to improve performances so we cannot have "everything" under normal maps.

Another thing that might be important : ordered maps requires some order criteria, for ints it is easy, for objects it must be done by the code. Actually I believe that most of our objects are "orderable" thanks to "Identifiable" template (please have a look into Object.h). This might (or might not) be what you need ?

ldallolio commented 7 years ago

I forgot one thing : objects are ordered thanks to "Identifiable", that uses "Reference" to compare between them (almost everything has a Reference here so it is quite general). Reference, if I recall correctly, has an auto_id : an automatically incremented id for each object type (one for analysis, one for constraints and so on). This might also be used to force some "insertion order" for free, I suppose.

ThomasAbballe commented 7 years ago

a Container has (not is) a lightweight ordered map "by_id"

Yes indeed. Container has a map, an unordered_map, an hash, a few functions to add/delete/print and that's it. That's why i tend to confuse the "containers" and the "contained", sorry for the mistake.

If I understand what you need, thought, you would like to keep "insertion order"

I want to have the exact same output from the same input, on every computer. And I also want not-impacted parts of the output to be exactly the same from one developments to an other. Insertion order can be a solution, of course, or something base on _autoid as you suggest.

Also, unordered maps are much faster than ordered ones, this is the reason we use them ;-) We wanted to improve performances so we cannot have "everything" under normal maps.

Is this really an issue with Containers ? From my experience, unlike cells or nodes, Containers have a hundred of objects, at worst. What order/percentage of "improve performance" are we talking about?

could you please check that a Container is really, as you say, unordered ?

Well, to be exact, "ordering problems" happen when VEGA writes Cell and Nodes groups, lists, vectors, and rbe nodes/elements (which are created by the Writer for a specific translation) into the Systus ASC file. Other parts (material, analysis) are fine. I investigate this a bit, and the only thing that point to this problem are the _unorderedmaps of Container. If it's not this, I am at loss here... As I said, we are threading very closely to the limit of my C++ knowledge ^^

I am sorry I cannot look in detail at this moment

Sure, no problem. It's not a priority for me neither ;)

ThomasAbballe commented 5 years ago

5a457c1813568a342056776d4a12372468432936 corrects this issue