New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RectPartition and Deformation fixes #748
Conversation
* Add domain parameter to LinDeformFixedTempl.__init__, enables different dtypes, backends and interpolations * Relax condition of coord space and template space in deformations to requiring that the partitions match.
acc9b7d
to
388bfcd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor things to fix.
"""Initialize a new instance. | ||
|
||
Parameters | ||
---------- | ||
template : `DiscreteLpElement` | ||
Fixed template that is to be deformed. | ||
domain : power space of `DiscreteLp` | ||
The space of all allowable coordinates in the deformation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allowable?
allowed I'd say
A `ProductSpace` of ``template.ndim`` copies of a function-space. | ||
It must fulfill | ||
``domain[0].partition == template.space.partition``, so | ||
this option is useful mainly when different interpolations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when using
"""Initialize a new instance. | ||
|
||
Parameters | ||
---------- | ||
template : `DiscreteLpElement` | ||
Fixed template that is to be deformed. | ||
domain : power space of `DiscreteLp` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional
It must fulfill | ||
``domain[0].partition == template.space.partition``, so | ||
this option is useful mainly when different interpolations | ||
in the displacement and template. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default?
domain = self.template.space.real_space.tangent_bundle | ||
else: | ||
if not isinstance(domain, ProductSpace): | ||
raise TypeError('`coord_space` must be a `ProductSpace` ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check error message
raise TypeError('`coord_space` must be a power space, ' | ||
'got {!r}'.format(domain)) | ||
if not isinstance(domain[0], DiscreteLp): | ||
raise TypeError('`coord_space[0]` must be a `DiscreteLp` ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check
'instance, got {!r}'.format(domain[0])) | ||
|
||
if template.space.partition != domain[0].partition: | ||
raise ValueError('`template.space.partition` not ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
break like this
raise ValueError(
'...')
That should require much less lines.
It must fulfill | ||
``domain[0].partition == template.space.partition``, so | ||
this option is useful mainly when different interpolations | ||
in the displacement and template. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mention complex spaces in the other variant but not here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not relevant here, giving a complex template is handled properly already without it.
raise ValueError('`templ_space.real_space.tangent_bundle` not ' | ||
'equal to `displacement.space` ({} != {})' | ||
if templ_space.partition != space[0].partition: | ||
raise ValueError('`templ_space.partition` not ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
option is useful mainly for support of complex spaces. | ||
``templ_space[0].partition == displacement.space.partition``, so | ||
this option is useful mainly for support of complex spaces and if | ||
different interpolations should be used for the displacement and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for the
Updated by comments. merge? |
Sure! |
Fixes issue #747,
RectPartition.__ne__
missing and also fixes what I was aiming to fix, more relaxed conditions for the template space and coordinate space.