CellComplex not recognized from IFC after changes introduced in v0.7.72

[Krande]

Hey,

I recently upgraded from topologicpy v0.7.65 to v0.7.77 and I found that most of my IFC's containing spatial volumes making up a cellcomplex no longer was recognized as a CellComplex.

I tracked down this change in behaviour to the release v0.7.72 https://github.com/wassimj/topologicpy/compare/v0.7.71...v0.7.72.

For reference I have included a reproducible example.

Here ia an example IFC file dev_topo_concept.ifc in a zip file dev_topo_concept.zip

The example IFC contains 2 IfcBlock geometries located next to eachother.

In topologicpy <v0.7.72 the following example produced a topologic_core.CellComplex comprised of 2 cells. In >=v0.7.72 it returns a topologic_core.Cluster with 0 cells.

import json
import pathlib

import ifcopenshell
import topologic_core
from topologicpy.Cell import Cell
from topologicpy.Cluster import Cluster
from topologicpy.Dictionary import Dictionary
from topologicpy.Topology import Topology


def get_ifc_metadata_from_cell(cell: Cell) -> dict:
    metadata = {}
    dictionary = cell.GetDictionary()
    keys = dictionary.Keys()
    for key in keys:
        try:
            attr = dictionary.ValueAtKey(key)
        except:
            raise Exception("Dictionary.Values - Error: Could not retrieve a Value at the specified key (" + key + ")")
        if isinstance(attr, topologic_core.IntAttribute):  # Hook to Core
            metadata[key] = attr.IntValue()
        elif isinstance(attr, topologic_core.DoubleAttribute):  # Hook to Core
            metadata[key] = attr.DoubleValue()
        elif isinstance(attr, topologic_core.StringAttribute):  # Hook to Core
            temp_str = attr.StringValue()
            if temp_str.startswith("{") and temp_str.endswith("}"):
                metadata[key] = json.loads(temp_str)
            elif temp_str == "__NONE__":
                metadata[key] = None
            else:
                metadata[key] = temp_str
        elif isinstance(attr, topologic_core.ListAttribute):  # Hook to Core
            metadata[key] = Dictionary.ListAttributeValues(attr)
        else:
            metadata[key] = ""

    return metadata


def main_example():
    result = Topology.ByIFCFile(ifcopenshell.open(pathlib.Path(__file__).parent / "dev_topo_concept.ifc"),
                                removeCoplanarFaces=True, transferDictionaries=True)
    topologies = list(filter(lambda x: x is not None, result))

    if not isinstance(topologies, list):
        raise ValueError(
            "Topology.MergeAll - Error: the input topologies parameter is not a valid list. Returning None."
        )
    other = []
    equipments = []
    topologies_filtered = []
    for t in topologies:
        if not Topology.IsInstance(t, "Topology"):
            continue
        meta = get_ifc_metadata_from_cell(t)
        ifc_function = meta.get("IFC_Properties", {}).get("IFC_function")
        if ifc_function is None:
            continue
        if ifc_function == "equipment":
            equipments.append(t)
        elif ifc_function == "space":
            topologies_filtered.append(t)
        else:
            other.append((ifc_function, t))

    if len(topologies_filtered) < 1:
        raise ValueError(
            "Topology.MergeAll - Error: the input topologyList does not contain any valid topologies. Returning None."
        )
    print(len(topologies_filtered), "spaces found.")
    cell_complex = Topology.SelfMerge(
        Cluster.ByTopologies(topologies_filtered, transferDictionaries=True), transferDictionaries=True
    )
    cells = Topology.Cells(cell_complex)
    print(f"{cells=}")
    if not isinstance(cell_complex, topologic_core.CellComplex):
        raise ValueError("Topology.MergeAll - Error: the output cellComplex is not a valid CellComplex.")

    print("CellComplex created successfully.")

if __name__ == '__main__':
    main_example()

I checked all environments from v0.7.65-0.7.72 and the latest 0.7.77.

Here are 2 conda env yml files which I used for topologicpy v0.7.71 and v0.7.72

# working.yml
name: topo_v71
channels:
  - conda-forge
dependencies:
  - python=3.12
  - ifcopenshell
  - pip
  - pip:
      - topologicpy==0.7.71

#not_working
name: topo_v72
channels:
  - conda-forge
dependencies:
  - python=3.12
  - ifcopenshell
  - pip
  - pip:
      - topologicpy==0.7.72

It definitively might be something wrong in my code, so please let me know if there's a change in behaviour between these versions that I need to account for.

Let me know if there's anything else I can help to debug this!

Best Regards Kristoffer

[Krande]

So I figured out the what caused the change in behaviour:

When iterating over the ifc shape geometries, the topologyType is no longer passed in as "CellComplex".

In my fork I made a new branch with a small change to revert this (and I added a unittest for this particular behaviour) See https://github.com/wassimj/topologicpy/compare/main...Krande:topologicpy:fix/cell-complex-from-ifc

The question is then what is the desired behaviour? If you think this is simply an unintended oversight, I can make PR with my suggestion if you'd like.

Let me know what you think,

Best Regards Kristoffer

[wassimj]

Dear @Krande Creating a CellComplex is one of the most expensive geometric and topological operations in #topologicpy. Many users don’t care about the actual entity type and just want to see the geometry (so can be just a soup of faces). This is why the default for Topology.ByGeometry is now a Cluster. Does your code work as expected if you pass “CellComplex” as a desired shape type? If yes, then I prefer to keep the code as is. If not, then let’s discuss further for me to better understand the problem. Thanks

[wassimj]

Hmmm. No sorry. I rushed that answer. The “CellComplex” is passed in MY code not yours. OK. I need to better design the input parameters to allow the user to either get the highest possible topology type or to be happy with just a cluster.

[Krande]

OK. I need to better design the input parameters to allow the user to either get the highest possible topology type or to be happy with just a cluster.

Having the ability to override the default behaviour by simply passing in a parameter sounds like a good idea!