Skip to content

intersect/average methods are a little awkward #233

@stephankramer

Description

@stephankramer

Discussion point: I find the way in which intersect (average) take the intersection (average) of self with its input arguments, and stores the result in self itself (i.e. overwrites in place) a little awkward to deal with.

In my typical setup I derive a number of individual metrics based on the Hessian of the fields (field components) I want to adapt to, which I then have as a list of metrics. Then I want to combine them in a single metric through intersection or averaging which currently leads to the following code:

   final_metric = metrics[0]
   final_metric.intersect(*metrics[1:])
   final_metric.rename('FinalMetric')
   final_metric.set_parameters(...)

which I think is a little opaque. If I want to keep metrics[0] - for instance I want to output the individual metrics along with the final metric for debugging - I first need to make a copy of the values of metrics[0] into the final metric

  final_metric = RiemannianMetric(TV, name='FinalMetric', metric_parameters=....)
  final_metric.assign(metrics[0])
  final_metric.intersect(*metrics[1:])

To me, it would be more natural if intersect would simply take the intersection of its input fields only, and store the result in self. In a way it feels a bit similar to implementing the __iadd__ operator before __add__. Also PETSc implements it with a separate output metric, so our in-place implementation actually still needs to make a copy.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions