yt-project / yt

Main yt repository
http://yt-project.org
Other
469 stars 280 forks source link

Optimize or eliminate calls to sanitize_edge #3179

Open matthewturk opened 3 years ago

matthewturk commented 3 years ago

In extracting ghost zones, a non-negligible amount of time is spent copying units and unit registries inside the function _sanitize_edge. This function looks like this:

    def _sanitize_edge(self, edge):
        if not is_sequence(edge):
            edge = [edge] * len(self.ds.domain_left_edge)
        if len(edge) != len(self.ds.domain_left_edge):
            raise RuntimeError(
                "Length of edges must match the dimensionality of the " "dataset"
            )
        if hasattr(edge, "units"):
            edge_units = edge.units.copy()
            edge_units.registry = self.ds.unit_registry
        else:
            edge_units = "code_length"
        return self.ds.arr(edge, edge_units, dtype="float64")

Copying a set of units can be quite costly, which seems to be where much of the time is spent. I don't completely understand why the various copies are necessary, but I suspect it was built up from a combination of issues that have cropped up in the past.

matthewturk commented 3 years ago

A little bit more to add to this -- in theory this is not necessary to call inside tight loops, like we get from generating ghost zones, because in those cases the left edge will always be created from previous, unitful objects from the same dataset. So in the case where we are calling it the most, it's not actually necessary.

neutrinoceros commented 3 years ago

I don't completely understand why the various copies are necessary

any chance this would be mitigated by https://github.com/yt-project/unyt/pull/199 ?

matthewturk commented 3 years ago

It is definitely possible, yes. But I am half-remembering that many of the deep copies were elsewhere.

neutrinoceros commented 1 year ago

This should be re-evaluated now that the aforementioned patch is released in unyt 2.9.x A reproducer script would be helpful.