ift issueshttps://gitlab.mpcdf.mpg.de/groups/ift/-/issues2018-01-30T20:41:01Zhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/214Gradients of all PowerEnergy classes and NoiseEnergy seem to be broken2018-01-30T20:41:01ZPhilipp Arrasparras@mpa-garching.mpg.deGradients of all PowerEnergy classes and NoiseEnergy seem to be brokenI was curious why my minimizations often produce overflows during the power spectrum estimation. Therefore, I wrote (partly together with @reimar) tests which compare the gradient with finite differences. It turns out that something is m...I was curious why my minimizations often produce overflows during the power spectrum estimation. Therefore, I wrote (partly together with @reimar) tests which compare the gradient with finite differences. It turns out that something is massively broken unless I overlook something.
In order to reproduce this phenomenon just check out the branch `energy_tests` and run `nosetests test/test_energies`. The mismatch is in one example -2.421884e+12 vs 48303.217449. That doesn't look good.
Since you have implemented the energies, @kjako, do you have any ideas what possibly goes wrong here?
The curvatures are not even tested yet...https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/213Can we avoid "inverse" FFT completely?2018-02-06T10:23:04ZMartin ReineckeCan we avoid "inverse" FFT completely?I just looked through all places in Nifty4 where we still use `FFTSmoothingOperator` (which implies the "inverse" harmonic transform which is not really defined on the sphere).
As far as I can tell, this is only really used in response ...I just looked through all places in Nifty4 where we still use `FFTSmoothingOperator` (which implies the "inverse" harmonic transform which is not really defined on the sphere).
As far as I can tell, this is only really used in response operators to simulate blurring by the instrument. Here, the inverse harmonic transform is used to go from position space to harmonic space before applying the convolution kernel which performs the smoothing. BUT, since we decided to reconstruct our signals in harmonic space, this operation is immediately preceded by _another_ harmonic transform which goes from harmonic space to position space! Typically, this happens in the form of
```
R = Instrument*fft
```
in our demo scripts.
So, I think we _do not_ need the inverse harmonic transform at all!
All we should do is design the `ResponseOperator` in a way that it directly goes from harmonic signal space to data space, and not (as it does at the moment) from positional signal space to data space. It will even make our codes faster, since we avoid an unnecessary pair of FFTs.
@ensslint, @parras, @reimar, @kjako, @dpumpe: is my reasoning OK or am I missing something?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/212NIFTy4: possible name changes?2018-06-19T09:27:38ZMartin ReineckeNIFTy4: possible name changes?I'll use this issue to collect a list of names that should probably changed before the NIFTy4 release, because the current names are confusing.
- The base object from which all unstructured and structured spaces are derived is called `D...I'll use this issue to collect a list of names that should probably changed before the NIFTy4 release, because the current names are confusing.
- The base object from which all unstructured and structured spaces are derived is called `DomainObject`. The total manifold on which a `Field` lives is called `DomainTuple`. Unstructured spaces have the type `FieldArray`, and structured ones are derived from `Space`. If we can make this less confusing, we definitely should ...
- `Field.dim` is not a very good name, in my opinion; it's too close to `numpy`'s `ndim`, which means something completely different. I'd suggest `size` or `npixels`.
- `FFTOperator` should probably be renamed to `HarmonicTransformOperator` or similar. FFT implies planar geometry and the existence of an inverse transform.
- and probably many more. Please feel free to add to this list!Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/211Question on generate_posterior_sample()2018-02-06T10:58:22ZMartin ReineckeQuestion on generate_posterior_sample()Currently the code for generate_posterior sample contains the line
`power = sqrt(power_analyze(self.S.diagonal()))`
According to @reimar, this should actually read:
`power = power_analyze(sqrt(self.S.diagonal()))`
@kjako, any objecti...Currently the code for generate_posterior sample contains the line
`power = sqrt(power_analyze(self.S.diagonal()))`
According to @reimar, this should actually read:
`power = power_analyze(sqrt(self.S.diagonal()))`
@kjako, any objections?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/210Should the "plotting" branch be merged?2018-02-01T10:55:38ZMartin ReineckeShould the "plotting" branch be merged?We have a branch called "plotting" which is 1 commit ahead of `master` and unchanged since 7 months.
@theos, @mbaltac, are there plans to merge it into master?We have a branch called "plotting" which is 1 commit ahead of `master` and unchanged since 7 months.
@theos, @mbaltac, are there plans to merge it into master?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/209Square root in Laplace operator2018-02-06T13:43:33ZPhilipp Arrasparras@mpa-garching.mpg.deSquare root in Laplace operatorCan some explain to me why we devide by the square root of the distances to compute the second derivative in the LaplaceOperator?
https://gitlab.mpcdf.mpg.de/ift/NIFTy/blob/nifty2go/nifty/operators/laplace_operator.py#L99
If I see it c...Can some explain to me why we devide by the square root of the distances to compute the second derivative in the LaplaceOperator?
https://gitlab.mpcdf.mpg.de/ift/NIFTy/blob/nifty2go/nifty/operators/laplace_operator.py#L99
If I see it correctly, it appears first in commit https://gitlab.mpcdf.mpg.de/ift/NIFTy/commit/05eeeabb347754445190675af2ab761f89a6ed60 by @theos. I do not see why this makes sense already because of the denominator's dimensions.Philipp Arrasparras@mpa-garching.mpg.dePhilipp Arrasparras@mpa-garching.mpg.dehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/208Field.__abs__ returns incorrect data type for complex fields2018-02-06T10:57:01ZMartin ReineckeField.__abs__ returns incorrect data type for complex fieldsCurrently, calling `abs(f)` on a complex-valued field `f` returns a complex-valued field, instead of a real-valued one. This is at odds with `numpy`'s behaviour.Currently, calling `abs(f)` on a complex-valued field `f` returns a complex-valued field, instead of a real-valued one. This is at odds with `numpy`'s behaviour.Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/207Meaning of Field methods mean(), var() and std() is unclear2018-03-23T08:24:14ZMartin ReineckeMeaning of Field methods mean(), var() and std() is unclearAre we sure that the `Field` reduction methods `mean()`, `var()` and `std()` are doing what we expect from them?
Currently, `mean()` computes a straightforward average over all entries, without taking volume factors into account. Is thi...Are we sure that the `Field` reduction methods `mean()`, `var()` and `std()` are doing what we expect from them?
Currently, `mean()` computes a straightforward average over all entries, without taking volume factors into account. Is this what we want? For `GLSpace`s, the result may not be the desired one.
@theos, @kjako, @dpumpe, @reimar: what are the official/intended semantics?Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/206vdot on subspaces2018-01-11T13:30:06ZPumpe, Daniel (dpumpe)vdot on subspacesHi Martin,
I just recognised that ift.Field.vdot does only work on 'whole' fields and does not yet support .vdot on subspaces.
```
In [1]: import nifty2go as ift
In [2]: x1 = ift.RGSpace(200)
In [3]: x2 = ift.RGSpace(150)
In [4]: ...Hi Martin,
I just recognised that ift.Field.vdot does only work on 'whole' fields and does not yet support .vdot on subspaces.
```
In [1]: import nifty2go as ift
In [2]: x1 = ift.RGSpace(200)
In [3]: x2 = ift.RGSpace(150)
In [4]: m = ift.Field((x1,x2), val=.5)
In [5]: m.vdot(m, spaces=1)
---------------------------------------------------------------------------
NotImplementedError Traceback (most recent call last)
<ipython-input-5-5c9f18522742> in <module>()
----> 1 m.vdot(m, spaces=1)
/Users/danielpumpe/CloudStation/dpumpe/construction/NIFTy_2go/lib/python2.7/site-packages/nifty2go-3.9.0-py2.7.egg/nifty2go/field.pyc in vdot(self, x, spaces)
339 return fct*dobj.vdot(y.val, x.val)
340
--> 341 raise NotImplementedError("special case for vdot not yet implemented")
342 active_axes = []
343 for i in spaces:
NotImplementedError: special case for vdot not yet implemented
```
What it be possible to provide this functionality?Martin ReineckeMartin Reinecke2018-01-11https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/205NIFTy2go: DirectSmoothingOperator missing2017-12-28T17:54:04ZPumpe, Daniel (dpumpe)NIFTy2go: DirectSmoothingOperator missingHi,
to make 'educated' initial guesses for D4PO it can be helpful to have a DirectSmoothingOperator. Unfortunately this Operator is missing in NIFTy2go. Is there any reason behind this?Hi,
to make 'educated' initial guesses for D4PO it can be helpful to have a DirectSmoothingOperator. Unfortunately this Operator is missing in NIFTy2go. Is there any reason behind this?Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/204NIFTy2go, FFTOperator2017-12-12T16:00:11ZPumpe, Daniel (dpumpe)NIFTy2go, FFTOperatorHi Martin,
unfortunately the FFTOperator on ProductSpaces does not work or am I missing something?
```
In [13]: x1 = ift.RGSpace(200)
In [14]: x2 = ift.RGSpace(200)
In [15]: k1 = x1.get_default_codomain()
In [16]: k2 = x2.get_defau...Hi Martin,
unfortunately the FFTOperator on ProductSpaces does not work or am I missing something?
```
In [13]: x1 = ift.RGSpace(200)
In [14]: x2 = ift.RGSpace(200)
In [15]: k1 = x1.get_default_codomain()
In [16]: k2 = x2.get_default_codomain()
In [17]: FFT = ift.FFTOperator(domain=(x1,x2), target=(k1, k2), space=1)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-17-da937f755b58> in <module>()
----> 1 FFT = ift.FFTOperator(domain=(x1,x2), target=(k1, k2), space=1)
/Users/danielpumpe/CloudStation/dpumpe/construction/NIFTy_2go/lib/python2.7/site-packages/nifty2go-3.9.0-py2.7.egg/nifty2go/operators/fft_operator.pyc in __init__(self, domain, target, space)
93 self._target = [dom for dom in self.domain]
94 self._target[self._space] = target
---> 95 self._target = DomainTuple.make(self._target)
96 adom.check_codomain(target)
97 target.check_codomain(adom)
/Users/danielpumpe/CloudStation/dpumpe/construction/NIFTy_2go/lib/python2.7/site-packages/nifty2go-3.9.0-py2.7.egg/nifty2go/domain_tuple.pyc in make(domain)
49 if isinstance(domain, DomainTuple):
50 return domain
---> 51 domain = DomainTuple._parse_domain(domain)
52 obj = DomainTuple._tupleCache.get(domain)
53 if obj is not None:
/Users/danielpumpe/CloudStation/dpumpe/construction/NIFTy_2go/lib/python2.7/site-packages/nifty2go-3.9.0-py2.7.egg/nifty2go/domain_tuple.pyc in _parse_domain(domain)
69 if not isinstance(d, DomainObject):
70 raise TypeError(
---> 71 "Given object contains something that is not an "
72 "instance of DomainObject class.")
73 return domain
TypeError: Given object contains something that is not an instance of DomainObject class.
```Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/203On nifty2go branch: Remove S-operator from NonlinearWF-Class constructor2018-01-26T11:01:48ZPhilipp Arrasparras@mpa-garching.mpg.deOn nifty2go branch: Remove S-operator from NonlinearWF-Class constructorSince the S-Operator is assumed to be an identity in the Fourier basis, it is not sensible to expose it to the user via the constructor. Do you agree?Since the S-Operator is assumed to be an identity in the Fourier basis, it is not sensible to expose it to the user via the constructor. Do you agree?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/202On nifty2go branch: Domain of signal fields2018-03-13T08:56:15ZPhilipp Arrasparras@mpa-garching.mpg.deOn nifty2go branch: Domain of signal fieldsI think there is an inconsistency in the current nifty2go branch. The WienerFilter and CriticalFilter classes assume the signal field to live in harmonic space whereas the Nonlinear* classes assume it to be in signal space.
What is the ...I think there is an inconsistency in the current nifty2go branch. The WienerFilter and CriticalFilter classes assume the signal field to live in harmonic space whereas the Nonlinear* classes assume it to be in signal space.
What is the reason for having implemented WienerFilterEnergy and Curvature this way? How do we resolve this inconsistency?https://gitlab.mpcdf.mpg.de/ift/pyHealpix/-/issues/2pybind11 must already be installed2018-01-19T08:58:18ZTheo Steiningerpybind11 must already be installedWithout pybind11 being already installed the installation terminates with
```
./pyHealpix.cc:30:31: fatal error: pybind11/pybind11.h: No such file or directory
#include <pybind11/pybind11.h>
```
Doing a ``pip install pybind11`` solves...Without pybind11 being already installed the installation terminates with
```
./pyHealpix.cc:30:31: fatal error: pybind11/pybind11.h: No such file or directory
#include <pybind11/pybind11.h>
```
Doing a ``pip install pybind11`` solves the problem.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/201Add preconditioning to all our minimizers?2018-01-26T15:01:10ZMartin ReineckeAdd preconditioning to all our minimizers?As far as I understand, it should be possible to speed up all our minimization algorithms (not ony ConjugateGradient) by using an adequate preconditioner if available.
Unfortunately this is not discussed in the Nocedal&Wright book, unle...As far as I understand, it should be possible to speed up all our minimization algorithms (not ony ConjugateGradient) by using an adequate preconditioner if available.
Unfortunately this is not discussed in the Nocedal&Wright book, unless I have missed something. So I'm looking for implementation hints.
@theos, @kjako, @reimar, any tips, links to papers etc. ?Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/200Make generate_posterior_sample() an attribute of WienerFilterCurvature?2018-02-06T09:34:51ZMartin ReineckeMake generate_posterior_sample() an attribute of WienerFilterCurvature?The method `generate_posterior_sample()` in `sugar.py` requires its second argument to be of type `WienerFilterCurvature` and is quite dependent on `WienerFilterCurvature`'s internal design (e.g. that it has members called S, R and N and...The method `generate_posterior_sample()` in `sugar.py` requires its second argument to be of type `WienerFilterCurvature` and is quite dependent on `WienerFilterCurvature`'s internal design (e.g. that it has members called S, R and N and what their exact types are).
I think it might be cleaner to move ths method into the class.
@kjako, @theos, what is your opinion?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/199Fairly urgent: decide on convention for diagonal in DiagonalOperator2017-11-16T10:02:14ZMartin ReineckeFairly urgent: decide on convention for diagonal in DiagonalOperatorRecently I removed the last remaining uses of the `bare` keyword in some methods of `DiagonalOperator`, and this change has now been merged into the `nightly` branch. The code now behaves as if `bare=False`, which was the default before....Recently I removed the last remaining uses of the `bare` keyword in some methods of `DiagonalOperator`, and this change has now been merged into the `nightly` branch. The code now behaves as if `bare=False`, which was the default before.
However Torsten argues that it would be more natural to behave as if `bare=True`.
Both is fine with me, but we need to decide very quickly, because the first people have started adapting to `nightly` and will be unhappy if they have to change their codes again!https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/198Design problem in InvertibleOperatorMixin2018-01-26T15:02:39ZMartin ReineckeDesign problem in InvertibleOperatorMixinThe constructor for `InvertibleOperatorMixin` currently allows specifying iteration starting values via `forward_x0` and `backward_x0`. This is problematic for several reasons:
- these vectors have to live over the full domain of a late...The constructor for `InvertibleOperatorMixin` currently allows specifying iteration starting values via `forward_x0` and `backward_x0`. This is problematic for several reasons:
- these vectors have to live over the full domain of a later operator application call, and thereby restrict the general applicability of the operator which is one of Nifty's current design goals.
- `forward_x0` is used in `times()` and `adjoint_inverse_times()` although it is most likely not a good guess for at least one of both. Analogously for `backward_x0`.
- A starting guess is directly coupled to a concrete argument for which the operator is invoked; it is not a property of the operator, but of the operator/argument combination. So if one sets up an `InvertibleOperatorMixin` using the currently available tools, it will only work well for a single specific field.
I would much prefer being able to supply a starting guess with every `times()` (or `adjoint_times()` etc.) call; I'm thinking about a clean approach for this. Suggestions are welcome!https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/197Error found by dimensional analysis2018-01-26T14:55:37ZMartin ReineckeError found by dimensional analysisThe test case below was distilled from `wiener_filter_via_curvature.py` by Torsten and me.
It indicates that there is a unit error in the way we compute the mock signal.
```
import numpy as np
import nifty as ift
import numericalunits a...The test case below was distilled from `wiener_filter_via_curvature.py` by Torsten and me.
It indicates that there is a unit error in the way we compute the mock signal.
```
import numpy as np
import nifty as ift
import numericalunits as nu
np.random.seed(43)
correlation_length = 0.05*nu.m
field_sigma = 2.* nu.K
response_sigma = 0.01*nu.m
# stupid power spectrum, but we only care about units now
def power_spectrum(k):
a = 4 * correlation_length * field_sigma**2
return a / (1 + k * correlation_length) ** 4
# Total side-length of the domain
L = 2.*nu.m
# Grid resolution (pixels per axis)
N_pixels = 512
signal_space = ift.RGSpace([N_pixels], distances=L/N_pixels)
harmonic_space = ift.FFTOperator.get_default_codomain(signal_space)
fft = ift.FFTOperator(harmonic_space, target=signal_space)
power_space = ift.PowerSpace(harmonic_space)
mock_power = ift.Field(power_space, val=power_spectrum(power_space.kindex))
mock_harmonic = mock_power.power_synthesize(real_signal=True)
mock_signal = fft(mock_harmonic)
print "signal mean (measured in K):", mock_signal.mean()/nu.K
print "signal mean (measured in K/sqrt(m)): ", mock_signal.mean()*np.sqrt(nu.m)/nu.K
```
If this code is run several times (I tested with the `nightly` branch), the first printed number changes, but the second one does not, which means that the unit for the mock_signal field is actually sqrt(m)/K, which is not the intended one.
My guess is that we either have a dimensional error in `power_spectrum()` or in `Field.power_synthesize()`.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/196What does Field.vdot() do if the spaces keyword is present?2018-01-11T13:30:42ZMartin ReineckeWhat does Field.vdot() do if the spaces keyword is present?What is the expected result if we call
`a.vdot(b,spaces=0)`
where a is a Field living on one space and b is a Field living on two spaces?
I'm asking because the documentation says that the result should be float or complex, but the co...What is the expected result if we call
`a.vdot(b,spaces=0)`
where a is a Field living on one space and b is a Field living on two spaces?
I'm asking because the documentation says that the result should be float or complex, but the code actually returns a Field object.
Is there an undocumented constraint that both Fields must have the same number of domains?