ift issueshttps://gitlab.mpcdf.mpg.de/groups/ift/-/issues2018-01-11T13:30:06Zhttps://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]: 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?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?2018-01-11Martin 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 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?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/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/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 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.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/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/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 objections?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/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 `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!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/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 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?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/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 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...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/215More generic posterior sampling2018-02-21T21:13:02ZMartin ReineckeMore generic posterior samplingIt was mentioned that the machinery for drawing posterior samples should be made more general, and should not only be available for `WienerFilterCurvature`.
I'd be happy to implement this, but I need a clear description of the required ingredients and the algorithm.
Concerning this functionality, there are currently several open questions:
- issue #211
- issue #200
- NIFTy4 currently has two implementations: `generate_posterior_sample()` and `generate_posterior_sample2()`, both in `nifty4/library/wiener_filter_curvature.py`. Which is the correct one?
@kjako, @reimar, @parras: please let's get together and sort this out soon!It was mentioned that the machinery for drawing posterior samples should be made more general, and should not only be available for `WienerFilterCurvature`.
I'd be happy to implement this, but I need a clear description of the required ingredients and the algorithm.
Concerning this functionality, there are currently several open questions:
- issue #211
- issue #200
- NIFTy4 currently has two implementations: `generate_posterior_sample()` and `generate_posterior_sample2()`, both in `nifty4/library/wiener_filter_curvature.py`. Which is the correct one?
@kjako, @reimar, @parras: please let's get together and sort this out soon!https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/216Print of UnstructuredDomains not working2018-02-06T09:26:55ZPhilipp Arrasparras@mpa-garching.mpg.dePrint of UnstructuredDomains not workingError Message: 'UnstructuredDomain' object has no attribute 'scalar_dvol'Error Message: 'UnstructuredDomain' object has no attribute 'scalar_dvol'https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/217Parallelism in `probe_with_posterior_samples`2018-03-01T22:50:58ZPhilipp Arrasparras@mpa-garching.mpg.deParallelism in `probe_with_posterior_samples`Can we draw these samples in parallel? @pfrank, could you perhaps contribute your implementation here?Can we draw these samples in parallel? @pfrank, could you perhaps contribute your implementation here?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/218Bug in `probe_with_posterior_samples`2018-02-05T13:30:18ZPhilipp Arrasparras@mpa-garching.mpg.deBug in `probe_with_posterior_samples`Please look at the output of `demos/wiener_filter_via_hamiltonian.py` in the branch `work_on_demos`. There is a huge discrepancy between `posterior_mean.png` and `reconstruction.png`.
The formulae in `StatCalculator` seem to be correct.Please look at the output of `demos/wiener_filter_via_hamiltonian.py` in the branch `work_on_demos`. There is a huge discrepancy between `posterior_mean.png` and `reconstruction.png`.
The formulae in `StatCalculator` seem to be correct.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/219Power space not working in more than 2 dim.2018-02-06T17:18:02ZMargret WesterkampPower space not working in more than 2 dim.For changes in `demos/wiener_filter_via_curvature.py` of the dimension from 2 to 3 comes the error:
```
Traceback (most recent call last):
File "wiener_filter_via_curvature.py", line 43, in <module>
power_space = ift.PowerSpace(harmonic_space)
File "/home/philipp/.local/lib/python3.6/site-packages/nifty4-3.9.0-py3.6.egg/nifty4/spaces/power_space.py", line 139, in __init__
File "/home/philipp/.local/lib/python3.6/site-packages/nifty4-3.9.0-py3.6.egg/nifty4/spaces/rg_space.py", line 93, in get_k_length_array
File "/home/philipp/.local/lib/python3.6/site-packages/nifty4-3.9.0-py3.6.egg/nifty4/field.py", line 77, in __init__
MemoryError
```
What is wrong here?For changes in `demos/wiener_filter_via_curvature.py` of the dimension from 2 to 3 comes the error:
```
Traceback (most recent call last):
File "wiener_filter_via_curvature.py", line 43, in <module>
power_space = ift.PowerSpace(harmonic_space)
File "/home/philipp/.local/lib/python3.6/site-packages/nifty4-3.9.0-py3.6.egg/nifty4/spaces/power_space.py", line 139, in __init__
File "/home/philipp/.local/lib/python3.6/site-packages/nifty4-3.9.0-py3.6.egg/nifty4/spaces/rg_space.py", line 93, in get_k_length_array
File "/home/philipp/.local/lib/python3.6/site-packages/nifty4-3.9.0-py3.6.egg/nifty4/field.py", line 77, in __init__
MemoryError
```
What is wrong here?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/220Link in docu broken2018-02-12T14:48:28ZPhilipp Arrasparras@mpa-garching.mpg.deLink in docu brokenAlso found by @wmarg: This link [here](https://gitlab.mpcdf.mpg.de/ift/NIFTy/blob/NIFTy_4/docs/source/start.rst#L19) is broken. Where should it actually point?Also found by @wmarg: This link [here](https://gitlab.mpcdf.mpg.de/ift/NIFTy/blob/NIFTy_4/docs/source/start.rst#L19) is broken. Where should it actually point?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/221DiagonalOperator diagonal() function as property?2018-02-17T13:58:55ZChristoph LienhardDiagonalOperator diagonal() function as property?The DiagonalOperator class has a function diagonal() returning the respective Field.
IMHO this should be a property, since e.g.
```
diagonal_val = operator.diagonal().val
```
just looks stupid (from a purely designy pythonic point of view).
The main philosophy I propose in this case is that from a class point of view 'diagonal' to DiagonalOperator is the same as 'val' to Field.
Following this the getter of the proposed property 'diagonal' should not return a copy of the diagonal (in contrast to what the current function diagonal() returns) since the property 'val' of Field does not do this either.The DiagonalOperator class has a function diagonal() returning the respective Field.
IMHO this should be a property, since e.g.
```
diagonal_val = operator.diagonal().val
```
just looks stupid (from a purely designy pythonic point of view).
The main philosophy I propose in this case is that from a class point of view 'diagonal' to DiagonalOperator is the same as 'val' to Field.
Following this the getter of the proposed property 'diagonal' should not return a copy of the diagonal (in contrast to what the current function diagonal() returns) since the property 'val' of Field does not do this either.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/222Bug in LaplaceOperator2018-02-21T21:21:01ZMartin ReineckeBug in LaplaceOperatorThe current version of `LaplaceOperator` has a bug that causes the input field to be modified whenever its `adjoint_times()` method is called.
The fix is trivial and will be applied in a few minutes.
@kjako, @reimar, @parras, @pfrank, @dpumpe, please check if this maybe fixes any mysterious problems you have encountered!The current version of `LaplaceOperator` has a bug that causes the input field to be modified whenever its `adjoint_times()` method is called.
The fix is trivial and will be applied in a few minutes.
@kjako, @reimar, @parras, @pfrank, @dpumpe, please check if this maybe fixes any mysterious problems you have encountered!https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/223power_synthesize() vs. draw_sample()2018-02-21T21:11:06ZMartin Reineckepower_synthesize() vs. draw_sample()Are there any fundamental differences between
- calling `power_synthesize(f)`, and
- calling `DiagonalOperator(diagonal=f).draw_sample()`?
I think we have some duplicate code here, which could be cleaned up.
Related question: does `draw_sample()` work if the operator has complex values on the diagonal?
@reimar, @kjako, @parras, any comments?Are there any fundamental differences between
- calling `power_synthesize(f)`, and
- calling `DiagonalOperator(diagonal=f).draw_sample()`?
I think we have some duplicate code here, which could be cleaned up.
Related question: does `draw_sample()` work if the operator has complex values on the diagonal?
@reimar, @kjako, @parras, any comments?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/224Add kwargs to plotting2018-03-01T20:29:55ZPhilipp Arrasparras@mpa-garching.mpg.deAdd kwargs to plottingCan we add `linewidth` and `alpha` to the kwargs supported by 1d-plotting routines? If so, I can do it.
Are there perhaps additional kwargs you guys need?Can we add `linewidth` and `alpha` to the kwargs supported by 1d-plotting routines? If so, I can do it.
Are there perhaps additional kwargs you guys need?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/225broken link in Readme2018-02-22T13:00:52ZSilvan Streitbroken link in ReadmeThe link to the informal introduction in https://gitlab.mpcdf.mpg.de/ift/NIFTy#first-steps is broken.The link to the informal introduction in https://gitlab.mpcdf.mpg.de/ift/NIFTy#first-steps is broken.