ift issueshttps://gitlab.mpcdf.mpg.de/groups/ift/-/issues2020-05-19T12:51:06Zhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/299Add possibility to log energies2020-05-19T12:51:06ZPhilipp Arrasparras@mpa-garching.mpg.deAdd possibility to log energiesI have implemented a method to log energies inside a IterationController such that they can be analyzed afterwards. @mtr how do you like this implementation? Do you think we could/should make this part of nifty? Or do you have other ideas how to approach the problem?
https://gitlab.mpcdf.mpg.de/ift/papers/rickandresolve/-/commit/8b0e15dc4a533dfba17f29361345309e336dfb8a
(Since this is part of an unpublished project, this link is only visible for ift group members. Sorry!)
@veberle @mtrI have implemented a method to log energies inside a IterationController such that they can be analyzed afterwards. @mtr how do you like this implementation? Do you think we could/should make this part of nifty? Or do you have other ideas how to approach the problem?
https://gitlab.mpcdf.mpg.de/ift/papers/rickandresolve/-/commit/8b0e15dc4a533dfba17f29361345309e336dfb8a
(Since this is part of an unpublished project, this link is only visible for ift group members. Sorry!)
@veberle @mtrhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/298Tests are taking too long2020-05-19T08:02:40ZMartin ReineckeTests are taking too longOn my laptop, the Nifty regression test are taking almost 10 minutes at the moment, which starts to get really problematic during development. Is there a chance of reducing test sizes and/or avoiding redundancies in the current tests?
The biggest problem by far is test_correlated_fields.py, which accounts for roughly 40% of the testing time. Next is test_jacobian.py with 20%; next is test_energy_gradients.py.
@parras, @pfrank, @phaimOn my laptop, the Nifty regression test are taking almost 10 minutes at the moment, which starts to get really problematic during development. Is there a chance of reducing test sizes and/or avoiding redundancies in the current tests?
The biggest problem by far is test_correlated_fields.py, which accounts for roughly 40% of the testing time. Next is test_jacobian.py with 20%; next is test_energy_gradients.py.
@parras, @pfrank, @phaimhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/297Really obsolete Nifty docs are still online2020-05-14T06:58:53ZMartin ReineckeReally obsolete Nifty docs are still onlineI searched online for "nifty documentation" today, and the second hit was
https://wwwmpa.mpa-garching.mpg.de/ift/nifty/start.html
Can we please remove these completely obsolete files from the public internet? Our potential users might be grateful :)I searched online for "nifty documentation" today, and the second hit was
https://wwwmpa.mpa-garching.mpg.de/ift/nifty/start.html
Can we please remove these completely obsolete files from the public internet? Our potential users might be grateful :)Torsten EnsslinTorsten Ensslinhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/296Releasing NIFTy 6?2020-05-22T07:46:13ZMartin ReineckeReleasing NIFTy 6?I have the impression that we have enough new features to justify a NIFTy 6 release.
Does anyone have feature suggestions which should go into the code before the release?I have the impression that we have enough new features to justify a NIFTy 6 release.
Does anyone have feature suggestions which should go into the code before the release?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/295Suggestion: Add dtype property to DomainTuple2020-04-26T15:24:18ZPhilipp Arrasparras@mpa-garching.mpg.deSuggestion: Add dtype property to DomainTupleI suggest to add a property called `dtype` to `DomainTuple`. When wrapping a numpy array as `Field` the constructor would check that the dtype of the array and the `DomainTuple` coincide. Functions like `from_random` do not need to take a dtype any more. In particular, sampling from e.g. `DiagonalOperator`s become unambiguous in terms of drawing either a real or a complex field. This in turn would simplify the KL because the `lh_sampling_dtype` would disappear.
I am not 100% sure how to treat `RGSpace.get_default_codomain()`. Would the default codomain of a real space be real again or complex? My suggestion is to add an optional argument to that function and take as default the same dtype as the original space.
Also I am not sure how to tell the `DomainTuple` its dtype. I suggest as an additional argument of `DomainTuple.make()`.
What do you think about that, @mtr @pfrank @reimar?I suggest to add a property called `dtype` to `DomainTuple`. When wrapping a numpy array as `Field` the constructor would check that the dtype of the array and the `DomainTuple` coincide. Functions like `from_random` do not need to take a dtype any more. In particular, sampling from e.g. `DiagonalOperator`s become unambiguous in terms of drawing either a real or a complex field. This in turn would simplify the KL because the `lh_sampling_dtype` would disappear.
I am not 100% sure how to treat `RGSpace.get_default_codomain()`. Would the default codomain of a real space be real again or complex? My suggestion is to add an optional argument to that function and take as default the same dtype as the original space.
Also I am not sure how to tell the `DomainTuple` its dtype. I suggest as an additional argument of `DomainTuple.make()`.
What do you think about that, @mtr @pfrank @reimar?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/294Potential confusion in the direction of FFTs2020-05-13T11:23:26ZMartin ReineckePotential confusion in the direction of FFTsIn the current implementation of FFTOperator (https://gitlab.mpcdf.mpg.de/ift/nifty/-/blob/NIFTy_6/nifty6/operators/harmonic_operators.py#L76) we have the code:
```
if x.domain[self._space].harmonic: # harmonic -> position
func = fft.fftn
fct = 1.
else:
func = fft.ifftn
fct = ncells
```
where `x` is the input field.
This looks exactly the wrong way around.
@parras, @pfrank, @kjako, @reimar, @veberle, what do you think?
Switching the FFT direction breaks almost none of our tests (only one, which already has a FIXME :).In the current implementation of FFTOperator (https://gitlab.mpcdf.mpg.de/ift/nifty/-/blob/NIFTy_6/nifty6/operators/harmonic_operators.py#L76) we have the code:
```
if x.domain[self._space].harmonic: # harmonic -> position
func = fft.fftn
fct = 1.
else:
func = fft.ifftn
fct = ncells
```
where `x` is the input field.
This looks exactly the wrong way around.
@parras, @pfrank, @kjako, @reimar, @veberle, what do you think?
Switching the FFT direction breaks almost none of our tests (only one, which already has a FIXME :).2020-05-13https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/293[GU] Simplify point-wise Field operations2020-04-14T06:46:41ZMartin Reinecke[GU] Simplify point-wise Field operationsCurrently, point-wise operations on Nifty objects are not really implemented in a beautiful way: they are injected via some nasty Python tricks into `Field`, `MultiField`, `Operator` and the `sugar` module, and a slightly more complicated variant is added to `Linearization`.
My plan is to have a single method in `Field`, `MultiField`, `Linearization` and `Operator`, which is called `ptw` (or `pointwise`), which takes a string describing the requested pointwise operation (like "sin", "exp", "one_over" etc.). Implementation of the actual operations would be fairly trivial via a dictionary mapping these names to their `numpy` equivalents, and also describing their derivatives via `numpy` or custom functions where necessary.
If we like, we can leave the global pointwise functions injected into `sugar.py` unchanged for aesthetic reasons :)
Opinions?Currently, point-wise operations on Nifty objects are not really implemented in a beautiful way: they are injected via some nasty Python tricks into `Field`, `MultiField`, `Operator` and the `sugar` module, and a slightly more complicated variant is added to `Linearization`.
My plan is to have a single method in `Field`, `MultiField`, `Linearization` and `Operator`, which is called `ptw` (or `pointwise`), which takes a string describing the requested pointwise operation (like "sin", "exp", "one_over" etc.). Implementation of the actual operations would be fairly trivial via a dictionary mapping these names to their `numpy` equivalents, and also describing their derivatives via `numpy` or custom functions where necessary.
If we like, we can leave the global pointwise functions injected into `sugar.py` unchanged for aesthetic reasons :)
Opinions?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/292Many Linearization methods explicitly expect Fields2020-04-03T07:53:56ZMartin ReineckeMany Linearization methods explicitly expect FieldsWhile working on Field/MultiField unification, I noticed that the largest part of `Linearization` members do not accept `MultiFields`; a clear example is `sinc`, which explicitly creates a `Field` for its output, but many other methods break in more obscure ways.
If we want unification, we need all `Linearization` methods to work on `Multifields`, which is quite a bit of tedious work. Any volunteers?While working on Field/MultiField unification, I noticed that the largest part of `Linearization` members do not accept `MultiFields`; a clear example is `sinc`, which explicitly creates a `Field` for its output, but many other methods break in more obscure ways.
If we want unification, we need all `Linearization` methods to work on `Multifields`, which is quite a bit of tedious work. Any volunteers?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/291Problematic MultiField methods2020-11-28T13:54:22ZMartin ReineckeProblematic MultiField methodsCurrently, `MultiField` has methods like `s_sum`, `clip`, and many transcendental functions. I'm wondering whether they make any sense: what's the point of computing the sum over all values in several fields ... or computing the sine of all field entries?
We currently use `MultiField`'s `norm` property in minimization, but given that the components of the `MultiField` may have vastly different scales, is this actually a clever thing to do? This basically means that we stop minimizing once the field component with the largest values has converged ... not necessarily what we want.
@parras, @pfrank, @reimar, @kjakoCurrently, `MultiField` has methods like `s_sum`, `clip`, and many transcendental functions. I'm wondering whether they make any sense: what's the point of computing the sum over all values in several fields ... or computing the sine of all field entries?
We currently use `MultiField`'s `norm` property in minimization, but given that the components of the `MultiField` may have vastly different scales, is this actually a clever thing to do? This basically means that we stop minimizing once the field component with the largest values has converged ... not necessarily what we want.
@parras, @pfrank, @reimar, @kjakohttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/290Data types for the Grand Unification2020-04-08T18:20:48ZMartin ReineckeData types for the Grand Unification```
Space:
structured or unstructured set of points (current NIFTy's "Domain")
SpaceTuple:
external product of zero or more Spaces (current NIFTy's "DomainTuple")
SpaceTuple = (Space, Space, ...)
SpaceTupleDict:
dictionary (with string keys) of one or more SpaceTuples
SpaceTupleDict = {name1: SpaceTuple1, name2: SpaceTuple2, ...}
(current NIFTy's "MultiDomain")
Domain:
This is an abstract concept. Can currently be represented by SpaceTuple or SpaceTupleDict
Special domains:
ScalarDomain: empty SpaceTuple
Operator:
- Operators take an Operand defined on an input domain, transform it in some way
and return a new Operand defined on a (possibly different) target domain.
- Operators can be concatenated, as long as the domains at the interface are
identical. The result is another Operator.
Operand:
- Operand objects represent fields and potentially their Jacobians and metrics.
- an Operand object can be asked for its "value" and (if configured accordingly)
its Jacobian. If the target domain of an Operand object is scalar, a metric
may also be available.
- Applying an Operator to an Operand object will always return another Operand object.
class Operator(object):
@property
def domain(self):
# return input domain
@property
def target(self):
# return output domain
def __call__(self, other):
# if isinstance(other, Operand) return an Operand object
# else return an Operator object
def __matmul__(self, other):
return self(other)
class LinearOperator(Operator):
# more or less analogous to the current LinearOperator
class Operand(object):
# this unifies current NIFTy's Field, MultiField, and Linearization classes
@property
def domain(self):
# if no Jacobian is present, return None, else the Jacobian's domain.
@property
def target(self):
# return the domain on which the value of the Operand is defined. This is
# also the Jacobian's target (if a Jacobian is defined)
@property
def val(self):
# return a low level data structure holding the actual values (currently numpy.ndarray
# or dictionary of numpy.ndarrays. Read-only.
def val_rw(self):
# return a writeable copy of `val`
def fld(self):
# return am Operand that only contains the value content of this object. Its Jacobian and
# potential higher derivatives will be `None`.
@property
def jac(self):
return a Jacobian LinearOperator if possible, else None
@property
def want_metric(self):
return True or False
@property
def metric(self):
if self.jacobian is None, raise an exception
if self.target is not ScalarDomain, raise an exception
if not self.want_metric, raise an exception
if metric cannot be computed, raise an exception
return metric
```
```
Space:
structured or unstructured set of points (current NIFTy's "Domain")
SpaceTuple:
external product of zero or more Spaces (current NIFTy's "DomainTuple")
SpaceTuple = (Space, Space, ...)
SpaceTupleDict:
dictionary (with string keys) of one or more SpaceTuples
SpaceTupleDict = {name1: SpaceTuple1, name2: SpaceTuple2, ...}
(current NIFTy's "MultiDomain")
Domain:
This is an abstract concept. Can currently be represented by SpaceTuple or SpaceTupleDict
Special domains:
ScalarDomain: empty SpaceTuple
Operator:
- Operators take an Operand defined on an input domain, transform it in some way
and return a new Operand defined on a (possibly different) target domain.
- Operators can be concatenated, as long as the domains at the interface are
identical. The result is another Operator.
Operand:
- Operand objects represent fields and potentially their Jacobians and metrics.
- an Operand object can be asked for its "value" and (if configured accordingly)
its Jacobian. If the target domain of an Operand object is scalar, a metric
may also be available.
- Applying an Operator to an Operand object will always return another Operand object.
class Operator(object):
@property
def domain(self):
# return input domain
@property
def target(self):
# return output domain
def __call__(self, other):
# if isinstance(other, Operand) return an Operand object
# else return an Operator object
def __matmul__(self, other):
return self(other)
class LinearOperator(Operator):
# more or less analogous to the current LinearOperator
class Operand(object):
# this unifies current NIFTy's Field, MultiField, and Linearization classes
@property
def domain(self):
# if no Jacobian is present, return None, else the Jacobian's domain.
@property
def target(self):
# return the domain on which the value of the Operand is defined. This is
# also the Jacobian's target (if a Jacobian is defined)
@property
def val(self):
# return a low level data structure holding the actual values (currently numpy.ndarray
# or dictionary of numpy.ndarrays. Read-only.
def val_rw(self):
# return a writeable copy of `val`
def fld(self):
# return am Operand that only contains the value content of this object. Its Jacobian and
# potential higher derivatives will be `None`.
@property
def jac(self):
return a Jacobian LinearOperator if possible, else None
@property
def want_metric(self):
return True or False
@property
def metric(self):
if self.jacobian is None, raise an exception
if self.target is not ScalarDomain, raise an exception
if not self.want_metric, raise an exception
if metric cannot be computed, raise an exception
return metric
```
https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/289Overflows after updating NIFTy2020-03-27T14:50:45ZPhilipp Arrasparras@mpa-garching.mpg.deOverflows after updating NIFTyWhen updating to the new interpolation scheme (https://gitlab.mpcdf.mpg.de/ift/nifty/commit/37a5691e3013ca36303ee4c3e78c74485f516793), I get overflows in the minimization where I did not get any before. I am confused since the both gradient tests work better than before and the accuracy of the approximation is better. Maybe the operator works better now so that actually an inconsistency in my model is triggered or so. I have the feeling that it is not the operator's fault... ;)
@gedenhof can you share your observations about the overflows? If it is the same commit as for me, then I will go ahead and try to assemble a minimal demonstration case but I think this might be a lot of work so maybe we can find the problem without that.
@gedenhof, @lerouWhen updating to the new interpolation scheme (https://gitlab.mpcdf.mpg.de/ift/nifty/commit/37a5691e3013ca36303ee4c3e78c74485f516793), I get overflows in the minimization where I did not get any before. I am confused since the both gradient tests work better than before and the accuracy of the approximation is better. Maybe the operator works better now so that actually an inconsistency in my model is triggered or so. I have the feeling that it is not the operator's fault... ;)
@gedenhof can you share your observations about the overflows? If it is the same commit as for me, then I will go ahead and try to assemble a minimal demonstration case but I think this might be a lot of work so maybe we can find the problem without that.
@gedenhof, @lerouhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/288Make convergence tests less fragile2020-03-22T13:06:44ZMartin ReineckeMake convergence tests less fragileThe switch to `numpy`'s new RNG interface has shown that some of our convergence and consistency tests are not very robust: in principle these tests should succeed for any random seed we use during the problem setup, but this is apparently not the case. We should have a closer look at the problematic tests and fix them accordingly.The switch to `numpy`'s new RNG interface has shown that some of our convergence and consistency tests are not very robust: in principle these tests should succeed for any random seed we use during the problem setup, but this is apparently not the case. We should have a closer look at the problematic tests and fix them accordingly.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/287Switch to new random interface of numpy2020-03-24T11:25:13ZPhilipp Arrasparras@mpa-garching.mpg.deSwitch to new random interface of numpy`np.random.seed` is legacy. The numpy docs say:
> The best practice is to **not** reseed a BitGenerator, rather to recreate a new one. This method is here for legacy reasons.
> This example demonstrates best practice.
```python
from numpy.random import MT19937
from numpy.random import RandomState, SeedSequence
rs = RandomState(MT19937(SeedSequence(123456789)))
# Later, you want to restart the stream
rs = RandomState(MT19937(SeedSequence(987654321)))
```
We might want to migrate eventually.`np.random.seed` is legacy. The numpy docs say:
> The best practice is to **not** reseed a BitGenerator, rather to recreate a new one. This method is here for legacy reasons.
> This example demonstrates best practice.
```python
from numpy.random import MT19937
from numpy.random import RandomState, SeedSequence
rs = RandomState(MT19937(SeedSequence(123456789)))
# Later, you want to restart the stream
rs = RandomState(MT19937(SeedSequence(987654321)))
```
We might want to migrate eventually.Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/286NIFTy grand unification: unify MultiFields and Fields2020-04-07T17:33:33ZMartin ReineckeNIFTy grand unification: unify MultiFields and Fields- all new fields have the internal structure of a MultiField
- a classic "standard" field is represented by a new field with a single key
that is the empty string
- Many of our operators work on part of a DomainTuple (e.g. FFTOperator).
Typically this is specified by passing the domain and additionally a "spaces"
argument, which is None, int of tupe of ints.
Since in the future every domain is a "multi-domain", this is no longer
sufficient: the partial domain must now contain an additional string defining
the name of the required field component. This requires an update
(and renaming) of "parse_spaces", "infer_space" etc.
Maybe it's good to introduce a new "PartialDomain" class which contains
* a string containing the desired field component, and
* an integer tuple containing the desired subspaces of that component
- "MultiField" will be renamed to "Field"; "Field" will probably be renamed
to some internal helper class or completely implemented within the new "Field".
- "MultiDomain" will be renamed to ???; "DomainTuple" will probably become
"_DomainTuple", i.e. it should not be directly accessed by external users.
- "makeField" and "makeDomain" become static "make" members of "Field" and
"Domain"- all new fields have the internal structure of a MultiField
- a classic "standard" field is represented by a new field with a single key
that is the empty string
- Many of our operators work on part of a DomainTuple (e.g. FFTOperator).
Typically this is specified by passing the domain and additionally a "spaces"
argument, which is None, int of tupe of ints.
Since in the future every domain is a "multi-domain", this is no longer
sufficient: the partial domain must now contain an additional string defining
the name of the required field component. This requires an update
(and renaming) of "parse_spaces", "infer_space" etc.
Maybe it's good to introduce a new "PartialDomain" class which contains
* a string containing the desired field component, and
* an integer tuple containing the desired subspaces of that component
- "MultiField" will be renamed to "Field"; "Field" will probably be renamed
to some internal helper class or completely implemented within the new "Field".
- "MultiDomain" will be renamed to ???; "DomainTuple" will probably become
"_DomainTuple", i.e. it should not be directly accessed by external users.
- "makeField" and "makeDomain" become static "make" members of "Field" and
"Domain"Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/285Performance of Operator Jacobians2020-03-10T10:32:22ZReimar H LeikePerformance of Operator JacobiansMany of our operators call the Jacobian of Linearizations more than once, leading to exponential performance losses in chains of operators, as uncovered by the test_for_performance_issues branch.
TODOS:
- [x] extend the tests to not only cover energy operators
- [x] fix all uncovered performance issuesMany of our operators call the Jacobian of Linearizations more than once, leading to exponential performance losses in chains of operators, as uncovered by the test_for_performance_issues branch.
TODOS:
- [x] extend the tests to not only cover energy operators
- [x] fix all uncovered performance issueshttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/284Inconsistency of .sum() for Fields and Linearizations2020-03-11T23:04:51ZPhilipp Arrasparras@mpa-garching.mpg.deInconsistency of .sum() for Fields and LinearizationsIs it wanted that `Linearization.sum()` returns a scalar Field but `Field.sum()` returns an actual scalar, i.e. in most situations a `float`? `MultiField.sum()` inherits the behaviour from `Field`.
Linearizations: https://gitlab.mpcdf.mpg.de/ift/nifty/blob/NIFTy_6/nifty6/linearization.py#L237
Fields: https://gitlab.mpcdf.mpg.de/ift/nifty/blob/NIFTy_6/nifty6/field.py#L382
MultiFields: https://gitlab.mpcdf.mpg.de/ift/nifty/blob/NIFTy_6/nifty6/multi_field.py#L169Is it wanted that `Linearization.sum()` returns a scalar Field but `Field.sum()` returns an actual scalar, i.e. in most situations a `float`? `MultiField.sum()` inherits the behaviour from `Field`.
Linearizations: https://gitlab.mpcdf.mpg.de/ift/nifty/blob/NIFTy_6/nifty6/linearization.py#L237
Fields: https://gitlab.mpcdf.mpg.de/ift/nifty/blob/NIFTy_6/nifty6/field.py#L382
MultiFields: https://gitlab.mpcdf.mpg.de/ift/nifty/blob/NIFTy_6/nifty6/multi_field.py#L169https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/283Reduce licensing boilerplate2020-05-13T11:36:01ZGordian EdenhoferReduce licensing boilerplateIf we really feel the need to put licensing boilerplate in every source file, let's at least make it concise by e.g. using https://spdx.org/ids.If we really feel the need to put licensing boilerplate in every source file, let's at least make it concise by e.g. using https://spdx.org/ids.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/282FieldZeroPadder2020-02-19T16:45:27ZVincent EberleFieldZeroPadderFieldZeropadding.adjoint doesn't work in 2 dimensions.FieldZeropadding.adjoint doesn't work in 2 dimensions.Philipp FrankPhilipp Frankhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/281[NIFTy6] Boost performance of Correlated Fields2019-12-06T13:53:49ZPhilipp Arrasparras@mpa-garching.mpg.de[NIFTy6] Boost performance of Correlated FieldsDo PowerDistributor only on one slice and copy it with ContractionOperator.adjoint afterwards. The power distributor appears to be the bottleneck of correlated field evaluations currently.Do PowerDistributor only on one slice and copy it with ContractionOperator.adjoint afterwards. The power distributor appears to be the bottleneck of correlated field evaluations currently.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/280[NIFTy6] Implement partial insert operator for multi fields2019-12-06T16:54:00ZPhilipp Arrasparras@mpa-garching.mpg.de[NIFTy6] Implement partial insert operator for multi fields