MAPPING

[artofscience]

No description provided.

[dirkmunro89]

Had a look. Dont feel I am in a position (yet) to do anything other than approve in principle, and hope to understand better when we discuss.

[MaxvdKolk]

Looks like a nice way to express the classes and the resulting implementations in change_of_variable.py are quite compact now. Would Linear and DQA replace the Taylor* classes, or the other way around?

I wonder if the eval function could be done slightly differently, but lets focus on those details later :].

We might also consider to add another function to make the __setindex__ a bit more discoverable as mm[[1], [2]] = Exp(-2) can be a bit dense. Maybe a wrapper like mm.set_mapping(response=[1], variables=[2], mapping=Exp(-2)) as well? (which would just forward to __setindex__ internally)

[MaxvdKolk]

Suggested change

	def __init__(self, mapping=None):
	self.map = mapping if mapping is not None else Linear()
	def __init__(self, mapping=Linear()):
	self.map = mapping

[MaxvdKolk]

For now it might be too soon, but might be good to write a brief set of docs on the interplay between self.map.update and self._update and the contents of the arguments :].

[MaxvdKolk]

Is ABC here mostly used to indicate this is the base class? There are no methods that are explicitly labelled with @abstracmethod or similar?

[MaxvdKolk]

Similar as a previous remark, not sure if the ABC is necessary, but could be kept in if needed (later on?).

[MaxvdKolk]

Would it make sense to split the key argument into two arguments to make it more clear both two entries need to be passed?

Suggested change

	def __setitem__(self, key, inter: Mapping):
	self.map.append((inter, key[0], key[1]))
	def __setitem__(self, response, variable, inter: Mapping):
	self.map.append((inter, response, variable))

[MaxvdKolk]

Would it make sense to have Sum as the default behaviour of TwoMap? What happens if someone uses TwoMap.g(..)? Or is that not intended behaviour?

[MaxvdKolk]

I think you might get the behaviour you are looking for by placing the code like this:

def update(self, x0, dg0, ddg0=0):
    if self.child is not None:
        # Traverse until inner-most child
        x0, dg0, ddg0 = self.child.update(x0, dg0, ddg0)
    
    # Bubble up the results from inside to outside,  and use the
    # updated (x0, dg0, ddg0) tuple from its inner mapping.
    return self._update(x0, dg0, ddg0)

def _update(self, x0, dg0, ddg0):
    # Mapping-specific version of the update rule that 
    # updates the self.g0, dg0, ddg0 attributes.
    return self.g0, self.dg0, self.ddg0