Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: astshim
-
Labels:
-
Story Points:0.5
-
Epic Link:
-
Sprint:AP S18-6
Description
PolyMap.polyTran replaces the forward or reverse coefficients with a fit. But if it is used to fit the inverse (as is typical) and the original PolyMap has an iterative inverse, the returned mapping will use that iterative inverse and ignore the fit coefficients.
I have asked David Berry if astPolyTran is behaving as it should by copying the value of IterInverse instead of clearing it. I am guessing it is intentional, in which case I can see three solutions:
- Have astshim clear IterInverse in the returned mapping when appropriate.
- Document the problem and allow PolyMap to be modified by setting IterInverse and, presumably, the two related properties, thus breaking the rule that mappings are immutable (other than Id and Ident). Allowing these properties to be set might be useful for tweaking behavior.
- Document the problem and be done with it. I feel this is too surprising to consider.
Of these I feel the first, clearing IterInverse when appropriate, is most appropriate and least surprising. However, it will require a bit of care, as the original mapping may be inverted and polyTran can fit either direction. I believe the logic is as follows:
If the mapping being fit has IterInverse set:
If the mapping being fit is not inverted and polyTran is fitting the inverse:
unset IterInverse in the returned mapping
if the mapping being fit is inverted and polyTran is fitting the forward direction:
unset IterInverse in the returned mapping
Which simplifies to:
If polyMap IterInverse is set and polyTran forward == PolyMap.isInverted():
unset IterInverse in the returned mapping
Independently it is worth considering allowing modifying the iterative inverse parameters but I'd like a clearer need before doing so.
David Berry modified AST to have astPolyTran clear the IterInverse flag.
Unfortunately he added a bug: he clears the flag if that direction is not the one being fit by PolyMap.polyTran, meaning the user is fitting the opposite to an iterative function, which is not a very reasonable thing to do anyway. I have chosen to raise an exception in that case.