Ifcopenshell: Improved IFC optimiser recipe

[Moult]

Ping @johltn - I read your article here on optimisation of IFCs: http://academy.ifcopenshell.org/optimize-ifc-files/

I previously wrote a very slow IfcPatch recipe that recycled non-rooted elements. I guess it also does optimisation, but is much slower compared to Solibri's results, although if you run it iteratively I did manage to get better than Solibri's results. Here's the code: https://github.com/IfcOpenShell/IfcOpenShell/blob/v0.6.0/src/ifcpatch/recipes/RecycleNonRootedElements.py

Would you be interested in converting your article into an IfcPatch recipe? This would allow users to run the optimisation code via the IfcPatch CLI, GUI, or as a library in their own app for convenience.

[johltn]

Yes sure good idea ! I can convert it to an IfcPatch recipe.

[Moult]

Awesome! Every patch recipe lives in https://github.com/IfcOpenShell/IfcOpenShell/tree/v0.6.0/src/ifcpatch/recipes and is a class called Patcher with a specific __init__ signature of __init__(self, src, file, logger, args=None) (in your case I suspect not much needs to be logged and no args are required).

https://github.com/IfcOpenShell/IfcOpenShell/blob/v0.6.0/src/ifcpatch/ifcpatch.py loads the relevant patch recipe and executes it.

When the recipe is ready let me know and I will bundle it with the GUI provided in Blender, so that graphical users can access it! I'll also document the instructions of running it in the OSArch wiki.

[johltn]

The recipe is ready on my fork https://github.com/johltn/IfcOpenShell/commit/99ce6dad5018202516c39b2ad84b862222a3a9ee.
To be used with python ifcpatch.py -i input.ifc -o optimised_file.ifc -r Optimise

[Moult]

Awesome @johltn ! Please feel free to commit it directly to this repo - I'll be happy to test and package it!

[johltn]

I don't have the authorization to push on the origin repo, but I can make a PR though

[Moult]

Merged the PR! Will update this issue when I've packaged it!

[Moult]

Hmm - I just tested it on a 109MB IFC file. I ran it for half an hour before quitting. From memory, I think Solibri was able to crunch a similar ~100MB (or more) file in under 10 minutes. This is similar to the performance issues I was having with the RecycleNonRootedElements recipe. In particular, I think the toposort() function itself takes ages.

Thoughts? Maybe a more low-tech approach is required?

[aothms]

I think the toposort() function itself takes ages.

I noticed we do a f.traverse(inst)[1:] so we find all recursive references. this is both expensive in IfcOpenShell and maybe (I don't know) also blows up the computation in toposort. Can you try with max_levels=1 on traverse()?

[Moult]

Hmm, I tried with max_levels=1 and it still took prohibitively long. @johltn what are the results like on your end? Maybe it's just me?

[johltn]

I've just run ifcpatch with max_levels=1 for a ~200MB file, more than 30 minutes ago, so I'm getting the same issue @Moult

[Moult]

@johltn perhaps you can look at my attempt in this recipe? https://github.com/IfcOpenShell/IfcOpenShell/blob/v0.6.0/src/ifcpatch/recipes/RecycleNonRootedElements.py It's much "dumber" and doesn't guarantee that a single iteration optimises the file fully, but is faster, at least... (not as fast as Solibri, though)

Maybe if this were written in C++ and if it used multicore it could be much faster?

[aothms]

I don't think there is something fundamental in the approach that makes it inefficient.

Looking at the profiling log on the execution I see most of the time is spent in get_info(). I got a bit of an improvement in by adding a @functools.cache decorator to the entity_instance.get_info definition. But I imagine a huge improvement would be not to make it recursive. It's recursive now so that instances with references to entity instances with the same values in the original file will be folded to the same instance and not only entity instances with the same values. But I think there is another way to accomplish that to make get_info not recursive and relying on the instance_mapping to unify return values.

~
85 1 44740302.0 44740302.0 1.7 iandr = dict(generate_instances_and_references())
86 38899 398290160.0 10239.1 15.3 for id in toposort(iandr):
87 38898 11545482.0 296.8 0.4 inst = f[id]
88 38898 1993730626.0 51255.4 76.8 info = inst.get_info(include_identifier=False, recursive=True, return_type=frozenset)
89 38898 3729792.0 95.9 0.1 if info in info_to_id:
90 11875 7580519.0 638.4 0.3 mapped = instance_mapping[inst] = instance_mapping[f[info_to_id[info]]]
91 else:
92 27023 1733791.0 64.2 0.1 info_to_id[info] = id
93 27023 1735268.0 64.2 0.1 instance_mapping[inst] = g.create_entity(
94 27023 3084362.0 114.1 0.1 inst.is_a(),
95 27023 92291733.0 3415.3 3.6 *map(map_value, inst)
96 )
97 1 11550491.0 11550491.0 0.4 g.write(sys.argv[1][:-4] + "_optimized.ifc")
~

[aothms]

Edit: another option would be to implement get_info (or the internal function there to create tuples of pairs) in C++.

[Moult]

Oh dear, I understood very little of that :) Especially this sentence:

It's recursive now so that instances with references to entity instances with the same values in the original file will be folded to the same instance and not only entity instances with the same values.

Is there something I can help code?

[aothms]

The problem is the following, let's say in case of

~~~

1=IFCCARTESIANPOINT((...));

2=IFCCARTESIANPOINT((...));

3=IFCCARTESIANPOINT((...));

4=IFCCARTESIANPOINT((...));

5=IFCPOLYLINE((#1,#2,#3,#4,#1));

~~~

toposort will get us to process the points first, but then when we process the polyline, get_info(recursive=True) will again visit all the points again to expand their attribute values into tuples. This repeats ad nauseam all the way up the graph 12 levels deep. This is not necessary, because due to the topological sort we already know which resources have been folded into the same instance so we can use that information rather than again and again expanding a tree of attribute values.

See the following sketch below, we add another use of map_value but now return some magic value for an instance reference with an id pointing to the instance in the new file. This allows us to use recursive=False.

~~~
instance_mapping = {}

def map_value(v, info=False):
    """
    Recursive function which replicates an entity instance, with 
    its attributes, mapping references to already registered
    instances. Indeed, because of the toposort we know that 
    forward attribute value instances are mapped before the instances
    that reference them.
    """
    if isinstance(v, (list, tuple)):
        return type(v)(map(functools.partial(map_value, info=info), v))
    elif isinstance(v, ifcopenshell.entity_instance):
        if info:
            if v.id() == 0:
                return ('__type__', v.is_a(), v[0])
            return ('__id__', instance_mapping[v].id())
        else:
            if v.id() == 0:
                return g.create_entity(v.is_a(), v[0])
            return instance_mapping[v]
    else:
        return v

info_to_id = {}
iandr = dict(generate_instances_and_references())
for id in toposort(iandr):
    inst = f[id]
    info = map_value(
        inst.get_info(include_identifier=False, recursive=False, return_type=tuple),
        info=True
    )
    ...

~~~

and now the profiling trace looks much more favourable and indeed most time is spent in toposort (this is after max_levels=1).

Perhaps somebody can have a look at whether the toposort implementation in e.g networkx is faster? https://networkx.org/documentation/stable/reference/algorithms/generated/networkx.algorithms.dag.topological_sort.html Although doesn't look promising, perhaps there are alternative implementations in C https://www.timlrx.com/2019/05/05/benchmark-of-popular-graph-network-packages/

    94         1         80.0     80.0      0.0      info_to_id = {}
    95         1   14072411.0 14072411.0      3.4      iandr = dict(generate_instances_and_references())
    96     38899  228162491.0   5865.5     54.7      for id in toposort(iandr):
    97     38898    6527704.0    167.8      1.6          inst = f[id]
    98     38898    1390319.0     35.7      0.3          info = map_value(
    99     38898   66217810.0   1702.3     15.9              inst.get_info(include_identifier=False, recursive=False, return_type=tuple),
   100     38898   21346384.0    548.8      5.1              info=True
   101                                                   )
   102     38898    1890305.0     48.6      0.5          if info in info_to_id:
   103     11875    3628009.0    305.5      0.9              mapped = instance_mapping[inst] = instance_mapping[f[info_to_id[info]]]
   104                                                   else:
   105     27023    1091938.0     40.4      0.3              info_to_id[info] = id
   106     27023     995415.0     36.8      0.2              instance_mapping[inst] = g.create_entity(
   107     27023    1726117.0     63.9      0.4                  inst.is_a(),
   108     27023   52735479.0   1951.5     12.6                  *map(map_value, inst)
   109                                                       )
   110         1    7101767.0 7101767.0      1.7      g.write(sys.argv[1][:-4] + "_optimized.ifc")

[aothms]

Edit2: perhaps https://igraph.org/python/doc/igraph.GraphBase-class.html#topological_sorting