MAPPING #140
MAPPING #140
Conversation
1. added empty map 2. implemented chain rule
Co-authored-by: max <MaxvdKolk@users.noreply.github.com>
…ine_approx_interv
Co-authored-by: max <MaxvdKolk@users.noreply.github.com>
dirkmunro89
left a comment
dirkmunro89
left a comment
There was a problem hiding this comment.
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
left a comment
MaxvdKolk
left a comment
There was a problem hiding this comment.
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)
sao/mappings/mapping.py
Outdated
| def __init__(self, mapping=None): | ||
| self.map = mapping if mapping is not None else Linear() |
There was a problem hiding this comment.
| def __init__(self, mapping=None): | |
| self.map = mapping if mapping is not None else Linear() | |
| def __init__(self, mapping=Linear()): | |
| self.map = mapping |
sao/mappings/mapping.py
Outdated
| def update(self, x0, dg0, ddg0=0): | ||
| self.map.update(x0, dg0, ddg0) | ||
| dg = self.map.dg(x0) | ||
| self._update(self.map.g(x0), dg0 / dg, ddg0 / dg ** 2 - dg0 * self.map.ddg(x0) / dg ** 3) |
There was a problem hiding this comment.
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 :].
| import numpy as np | ||
|
|
||
|
|
||
| class Mapping(ABC): |
There was a problem hiding this comment.
Is ABC here mostly used to indicate this is the base class? There are no methods that are explicitly labelled with @abstracmethod or similar?
| def _ddg(self, x): return np.zeros_like(x) | ||
|
|
||
|
|
||
| class Linear(Mapping, ABC): |
There was a problem hiding this comment.
Similar as a previous remark, not sure if the ABC is necessary, but could be kept in if needed (later on?).
| def __setitem__(self, key, inter: Mapping): | ||
| self.map.append((inter, key[0], key[1])) |
There was a problem hiding this comment.
Would it make sense to split the key argument into two arguments to make it more clear both two entries need to be passed?
| 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)) |
| class Sum(TwoMap): | ||
| def g(self, x): return self.left.g(x) + self.right.g(x) | ||
|
|
||
| def dg(self, x): return self.left.dg(x) + self.right.dg(x) | ||
|
|
||
| def ddg(self, x): return self.left.ddg(x) + self.right.ddg(x) |
There was a problem hiding this comment.
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?
sao/mappings/mapping.py
Outdated
| def update(self, x0, dg0, ddg0=0): | ||
| if self.child is not None and self.child.updated is False: # climb down to youngest child | ||
| self.child.update(x0, dg0, ddg0) | ||
| else: # climb up to oldest broer while updating the mappings | ||
| self._update(x0, dg0, ddg0) | ||
| self.updated = True | ||
| self.parent.update(self.g(x0), self.dg(x0), self.ddg(x0)) |
There was a problem hiding this comment.
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.ddg0Add tests for mapping
No description provided.