`restrict_anisotropy` arguement conflicts with `self._restrict_anisotropy_first`?

[acse-ej321]

animate/animate/metric.py

Lines 536 to 587 in cc40cd8

	restrict_anisotropy=True,
	):
	"""
	Apply :math:`L^p` normalisation to the metric.
	See
	https://mesh-adaptation.github.io/docs/animate/1-metric-based.html#metric-normalisation
	for more details.
	:kwarg global_factor: pre-computed global normalisation factor
	:type global_factor: :class:`float`
	:kwarg boundary: is the normalisation to be done over the boundary?
	:type boundary: :class:`bool`
	:kwarg restrict_sizes: should minimum and maximum metric magnitudes be enforced?
	:type restrict_sizes: :class:`bool`
	:kwarg restrict_anisotropy: should maximum anisotropy be enforced?
	:type restrict_anisotropy: :class:`bool`
	:return: the normalised metric, modified in-place
	:rtype: :class:`~.RiemannianMetric`
	"""
	d = self._tdim - 1 if boundary else self._tdim
	p = self.metric_parameters.get("dm_plex_metric_p", 1.0)
	target = self.metric_parameters.get("dm_plex_metric_target_complexity")
	if target is None:
	raise ValueError("dm_plex_metric_target_complexity must be set.")
	# Enforce that the metric is SPD
	self.enforce_spd(
	restrict_sizes=False, restrict_anisotropy=self._restrict_anisotropy_first
	)
	# Compute global normalisation factor
	detM = ufl.det(self)
	if global_factor is None:
	dX = (ufl.ds if boundary else ufl.dx)(domain=self._mesh)
	exponent = 0.5 if np.isinf(p) else (p / (2 * p + d))
	integral = firedrake.assemble(pow(detM, exponent) * dX)
	global_factor = firedrake.Constant(pow(target / integral, 2 / d))
	# Normalise the metric
	if boundary:
	raise NotImplementedError(
	"Normalisation on the boundary not yet implemented."
	)
	determinant = 1 if np.isinf(p) else pow(detM, -1 / (2 * p + d))
	self.interpolate(global_factor * determinant * self)
	# Enforce element constraints
	return self.enforce_spd(
	restrict_sizes=restrict_sizes,
	restrict_anisotropy=not self._restrict_anisotropy_first,
	)

The restrict_anisotropy argument in the :function:enforce_spd method of the :class:RiemannianMetric conflicts with the self._restrict_anisotropy_first variable in the :function:normalise method. It is unclear whether restrict_anisotropy is still necessary or has been replaced by self._restrict_anisotropy_first.

[acse-ej321]

animate/animate/metric.py

Lines 160 to 170 in cc40cd8

	for key in ("h_min", "h_max", "a_max"):
	key = f"dm_plex_metric_{key}"
	value = mp.get(key)
	if not value:
	continue
	if isinstance(value, (firedrake.Constant, ffunc.Function)):
	vp[key] = value
	mp.pop(key)
	self._variable_parameters_set = True
	else:
	vp[key] = firedrake.Constant(value)

In connection, I was wondering if self._variable_parameters_set = True should be added to the else clause in _process_parameters?

[joewallwork]

Well restrict_anisotropy_first=True doesn't necessarily imply restrict_anisotropy=True in the current implementation. It's an ordering thing. We could change the default parameter value to restrict_anisotropy_first=None and then only make it True if the user sets restrict_anisotropy=True, if that would be clearer?

[joewallwork]

In connection, I was wondering if self._variable_parameters_set = True should be added to the else clause in _process_parameters?

But isn't that variable marking that variable parameters (i.e., Function parameters) have been set?

[acse-ej321]

In connection, I was wondering if self._variable_parameters_set = True should be added to the else clause in _process_parameters?

But isn't that variable marking that variable parameters (i.e., Function parameters) have been set?

@jwallwork23 - in the else clause, is the line vp[key] = firedrake.Constant(value) doing something other than setting the Function parameters for the variable parameters?

[joewallwork]

In connection, I was wondering if self._variable_parameters_set = True should be added to the else clause in _process_parameters?

But isn't that variable marking that variable parameters (i.e., Function parameters) have been set?

@jwallwork23 - in the else clause, is the line vp[key] = firedrake.Constant(value) doing something other than setting the Function parameters for the variable parameters?

Unfortunately, we didn't leave a comment to remind about this. (We should add one.) I imagine that if one or more variable parameters are specified then the other parameters that could be variable but aren't are set as Constants that get passed to the variable version of the enforcement method.