ift issueshttps://gitlab.mpcdf.mpg.de/groups/ift/-/issues2018-06-27T15:54:46Zhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/234Documentation for our volume factor approach?2018-06-27T15:54:46ZMartin ReineckeDocumentation for our volume factor approach?Do we have a short document describing why (and how) our approach with almost no volume factors works?
Vanessa Boehm asked me for an explanation, which I can't give, and I expect more requests of that sort.
@reimar, would it be possible to add something to our documentation?Do we have a short document describing why (and how) our approach with almost no volume factors works?
Vanessa Boehm asked me for an explanation, which I can't give, and I expect more requests of that sort.
@reimar, would it be possible to add something to our documentation?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/244Update to NIFTy_52018-06-27T15:54:39ZPhilipp Arrasparras@mpa-garching.mpg.deUpdate to NIFTy_5- [x] Create NIFTy fork (@mtr @parras)
- [ ] Move KL and poisson demo from GlobalNewton to NIFTy fork. (@parras)
- [x] Revert documentation to NIFTy4 (@parras)
Use `Model` for constructing the following energies (@mtr, @jruestig, @reimar, @pfrank):
- [ ] `GaussianLH`, `PoissonianLH` (new curvature)
- [ ] `NoiseModel`
- [ ] `make_correlated_field` (move from GlobalNewton)
- [ ] `make_amplitude_model` (move from GlobalNewton)
- [ ] `make_multifrequency_correlated_field` / `separable_correlations`(move from GlobalNewton and adopt)
To this end one probably needs to implement a Gaussian and a Poissonian likelihood which is to be used in all energies (see GlobalNewton repository). Please add documentation directly.
Documentation:
- [ ] Documentation on volume factors (see also #234, @reimar, @maxn).
- [ ] Add Models (@hutsch) and MultiFields (@mtr ) to top-level documentation.
- [ ] `__add__` and ` __sub__` of `Energy` (@mtr)
- [ ] `Constant` (@jruestig, @clienhar)
- [ ] `LocalModel` (@jruestig, @clienhar)
- [ ] `Model` (@jruestig, @clienhar)
- [ ] `Add` in models (@jruestig, @clienhar)
- [ ] `ScalarMul` (@jruestig, @clienhar)
- [ ] `LinearModel` (@jruestig, @clienhar)
- [ ] `Variable` (@jruestig, @clienhar)
- [ ] `ModelGradientOperator` (@clienhar)
- [ ] `SamplingEnabler` (@kjako)
- [ ] `SelectionOperator` (@lplatz)
Tests:
- [ ] Consistency checks for all linear operators (see `ift.extra`!). (@lplatz, @silvan, @mawandro)
- [ ] Gradient consistency checks for all energies in library (see `ift.extra`, do that when the energies are rewritten). (@lplatz, @silvan, @mawandro)
- [ ] Unit tests for all new functionality, especially for models. (@parras, @jruestig)
- [ ] Write `ift.extra.model_consistency_check` and check every model with it (@reimar, @silvan)
Demos:
- [X] Fix `demos/poisson_demo.py` or move poisson demo from GlobalNewton here.
- [X] Think about concept for demos. Which ones? How universal? Which features? MAP solution for multi-problems until Jakob's paper is out?
- [ ] Write demos (first `demo/advanced.py`, then `getting_started_123`, 1: R=mask, WF, 2: R=mask, PoissonLogNormal, 3: R=LOS, PosTanh, Gauss, AmplitudeReconstruction). Start when energies are ready. (@kjako, @natalia)
Internals:
- [ ] Add something like a `CurvatureInversionEnabler` class, which takes an `Energy`, an `IterationController` and (optionally) a preconditioner and adds invertability to the energy's curvature operator. Having such a class would probably allow for some code reductions in many energy classes. (This is now tracked as issue #246). (@mtr)
- [x] Move classes out of `model.py` except `Model`.
- [ ] Revert automatical casting of `Field` to `Model`. (@parras)
- [ ] Rewrite `__add__` of `Energy` such that no trees are created (@mtr).
To be discussed:
- [X] Automatically cast `Field` to `Model` when multiplied with a `Model`?- [x] Create NIFTy fork (@mtr @parras)
- [ ] Move KL and poisson demo from GlobalNewton to NIFTy fork. (@parras)
- [x] Revert documentation to NIFTy4 (@parras)
Use `Model` for constructing the following energies (@mtr, @jruestig, @reimar, @pfrank):
- [ ] `GaussianLH`, `PoissonianLH` (new curvature)
- [ ] `NoiseModel`
- [ ] `make_correlated_field` (move from GlobalNewton)
- [ ] `make_amplitude_model` (move from GlobalNewton)
- [ ] `make_multifrequency_correlated_field` / `separable_correlations`(move from GlobalNewton and adopt)
To this end one probably needs to implement a Gaussian and a Poissonian likelihood which is to be used in all energies (see GlobalNewton repository). Please add documentation directly.
Documentation:
- [ ] Documentation on volume factors (see also #234, @reimar, @maxn).
- [ ] Add Models (@hutsch) and MultiFields (@mtr ) to top-level documentation.
- [ ] `__add__` and ` __sub__` of `Energy` (@mtr)
- [ ] `Constant` (@jruestig, @clienhar)
- [ ] `LocalModel` (@jruestig, @clienhar)
- [ ] `Model` (@jruestig, @clienhar)
- [ ] `Add` in models (@jruestig, @clienhar)
- [ ] `ScalarMul` (@jruestig, @clienhar)
- [ ] `LinearModel` (@jruestig, @clienhar)
- [ ] `Variable` (@jruestig, @clienhar)
- [ ] `ModelGradientOperator` (@clienhar)
- [ ] `SamplingEnabler` (@kjako)
- [ ] `SelectionOperator` (@lplatz)
Tests:
- [ ] Consistency checks for all linear operators (see `ift.extra`!). (@lplatz, @silvan, @mawandro)
- [ ] Gradient consistency checks for all energies in library (see `ift.extra`, do that when the energies are rewritten). (@lplatz, @silvan, @mawandro)
- [ ] Unit tests for all new functionality, especially for models. (@parras, @jruestig)
- [ ] Write `ift.extra.model_consistency_check` and check every model with it (@reimar, @silvan)
Demos:
- [X] Fix `demos/poisson_demo.py` or move poisson demo from GlobalNewton here.
- [X] Think about concept for demos. Which ones? How universal? Which features? MAP solution for multi-problems until Jakob's paper is out?
- [ ] Write demos (first `demo/advanced.py`, then `getting_started_123`, 1: R=mask, WF, 2: R=mask, PoissonLogNormal, 3: R=LOS, PosTanh, Gauss, AmplitudeReconstruction). Start when energies are ready. (@kjako, @natalia)
Internals:
- [ ] Add something like a `CurvatureInversionEnabler` class, which takes an `Energy`, an `IterationController` and (optionally) a preconditioner and adds invertability to the energy's curvature operator. Having such a class would probably allow for some code reductions in many energy classes. (This is now tracked as issue #246). (@mtr)
- [x] Move classes out of `model.py` except `Model`.
- [ ] Revert automatical casting of `Field` to `Model`. (@parras)
- [ ] Rewrite `__add__` of `Energy` such that no trees are created (@mtr).
To be discussed:
- [X] Automatically cast `Field` to `Model` when multiplied with a `Model`?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/145Add documentation to classes in library2018-06-27T15:53:10ZTheo SteiningerAdd documentation to classes in libraryThe `nifty2go` branch contains a great set of operator and energy objects. Please add proper docstrings to the classes and a reference to a paper/book where the respective algorithm is described. If there is no such paper where the implemented formulae can be found 1:1, please add inline comments on what is happening.The `nifty2go` branch contains a great set of operator and energy objects. Please add proper docstrings to the classes and a reference to a paper/book where the respective algorithm is described. If there is no such paper where the implemented formulae can be found 1:1, please add inline comments on what is happening.Jakob KnollmuellerJakob Knollmuellerhttps://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/243feature request: fun with multi2018-06-15T05:40:25ZReimar Heinrich Leikefeature request: fun with multiMultifields and operators do not yet support some of the convenience functionality present for normal fields and operators. For example a MultiField that is multiplied with a BlockDiagonalOperator is not automatically converted to a Diagonal Operator the way it is done for fields. Minimal example attached.
[multi_issue.py](/uploads/f634d63059005da5f20731d673373e8c/multi_issue.py)Multifields and operators do not yet support some of the convenience functionality present for normal fields and operators. For example a MultiField that is multiplied with a BlockDiagonalOperator is not automatically converted to a Diagonal Operator the way it is done for fields. Minimal example attached.
[multi_issue.py](/uploads/f634d63059005da5f20731d673373e8c/multi_issue.py)Martin ReineckeMartin Reineckehttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/242Sandwich does not like Field buns2018-05-29T12:35:17ZJakob KnollmuellerSandwich does not like Field bunsHi,
The SandwichOperator does not accept Fields as bun, as they do not have an .adjoint method. I propose either to allow Fields and cast them to Diagonal operators or to check for them in the init.Hi,
The SandwichOperator does not accept Fields as bun, as they do not have an .adjoint method. I propose either to allow Fields and cast them to Diagonal operators or to check for them in the init.https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/239Branch cleanup2018-05-23T12:36:47ZMartin ReineckeBranch cleanupI'd like to cleanup the branches which are no longer used. Could you please give me feedback which ones of your branches are still needed?
@reimar: easy_samples
Another question: should we merge the `working_on_demos` branch?I'd like to cleanup the branches which are no longer used. Could you please give me feedback which ones of your branches are still needed?
@reimar: easy_samples
Another question: should we merge the `working_on_demos` branch?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/240Drawing random fields in multi domains2018-05-23T10:15:45ZPhilipp Arrasparras@mpa-garching.mpg.deDrawing random fields in multi domainsThe following command breaks:
```
ift.Field.from_random("normal", ift.MultiDomain.make({'a': ift.RGSpace(1)}))
```
How can I fix that? Or is it not implemented yet?The following command breaks:
```
ift.Field.from_random("normal", ift.MultiDomain.make({'a': ift.RGSpace(1)}))
```
How can I fix that? Or is it not implemented yet?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/238extra.check_value_gradient_consistency does not work for MultiField2018-05-23T09:22:38ZJakob Knollmuellerextra.check_value_gradient_consistency does not work for MultiFieldextra.check_value_gradient_consistency calls Field.from_random, which does not support drawing random MultiFields.
Does it make more sense to remove the static Field.from_random, which always requires an additional domain, and instead introduce something like domain.random(random_type)? Alternatively one has to include checks for MultiField or regular Field.
Jakobextra.check_value_gradient_consistency calls Field.from_random, which does not support drawing random MultiFields.
Does it make more sense to remove the static Field.from_random, which always requires an additional domain, and instead introduce something like domain.random(random_type)? Alternatively one has to include checks for MultiField or regular Field.
Jakobhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/237Comparing Fields to floats2018-05-16T10:08:38ZPhilipp Arrasparras@mpa-garching.mpg.deComparing Fields to floatsIs the following behaviour intended?
```
import nifty4 as ift
f = ift.Field.full(ift.RGSpace(1), 1.)
if f == 0.:
print(f==0.)
```
Results in:
```
nifty4.Field instance
- domain = DomainTuple, len: 1
RGSpace(shape=(1,), distances=(1.0,), harmonic=False)
- val = array([False])
```Is the following behaviour intended?
```
import nifty4 as ift
f = ift.Field.full(ift.RGSpace(1), 1.)
if f == 0.:
print(f==0.)
```
Results in:
```
nifty4.Field instance
- domain = DomainTuple, len: 1
RGSpace(shape=(1,), distances=(1.0,), harmonic=False)
- val = array([False])
```https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/236try except in InverseEnabler.draw_sample2018-05-02T08:01:18ZChristoph Lienhardtry except in InverseEnabler.draw_samplejust catching every error in the `draw_sample` function of the `InversionEnabler` class might shadow 'real' errors.
IMO only `ValueError`s should be excepted.
(just had a problem and only got the 'cannot draw from inverse of this operator', which is weird, if the InversionEnabler is on. Also I was not drawing from the inverse.
The reason was, that the `try` clause caught a completely different error jumped into `except` where it tried to draw a sample from the inverse of said operator, which was not implemented for that operator)just catching every error in the `draw_sample` function of the `InversionEnabler` class might shadow 'real' errors.
IMO only `ValueError`s should be excepted.
(just had a problem and only got the 'cannot draw from inverse of this operator', which is weird, if the InversionEnabler is on. Also I was not drawing from the inverse.
The reason was, that the `try` clause caught a completely different error jumped into `except` where it tried to draw a sample from the inverse of said operator, which was not implemented for that operator)https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/232Random seed in extra/tests2018-04-26T15:01:09ZPhilipp Arrasparras@mpa-garching.mpg.deRandom seed in extra/testsIf one calls tests like `nifty4.extra.consistency_check(op)` the random seed afterwards is different. Would it be sensible to reset the numpy random seed?If one calls tests like `nifty4.extra.consistency_check(op)` the random seed afterwards is different. Would it be sensible to reset the numpy random seed?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/233Incorrect Energy.curvature() call in minimizers/conjugate_gradient.py?2018-04-20T13:47:34ZLukas PlatzIncorrect Energy.curvature() call in minimizers/conjugate_gradient.py?In [minimization/conjugate_gradient.py, line 76](https://gitlab.mpcdf.mpg.de/ift/NIFTy/blob/NIFTy_4/nifty4/minimization/conjugate_gradient.py#L76) there is a call to 'energy.curvature(d)'.
However all energies defined in NIFTy only have argument-less curvature methods.
Still, the ConjugateGradient Class is usable without error messages. What am I missing?In [minimization/conjugate_gradient.py, line 76](https://gitlab.mpcdf.mpg.de/ift/NIFTy/blob/NIFTy_4/nifty4/minimization/conjugate_gradient.py#L76) there is a call to 'energy.curvature(d)'.
However all energies defined in NIFTy only have argument-less curvature methods.
Still, the ConjugateGradient Class is usable without error messages. What am I missing?https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/227Renaming 'structured' and 'unstructered' domains.2018-04-06T11:24:55ZTheo SteiningerRenaming 'structured' and 'unstructered' domains.In my opinion the wording of 'structured' and 'unstructured' domains is misleading. The 'unstructured' domains can have plenty of structure, too, e.g. a vector-field. Mathematically, 'structured' domains are manifolds and 'unstructured' domains are fibers/Fasern that are defined on top of the product of all manifolds. Together, everything forms a fiber-bundle.
See, for reference:
https://en.wikipedia.org/wiki/Fiber_bundleIn my opinion the wording of 'structured' and 'unstructured' domains is misleading. The 'unstructured' domains can have plenty of structure, too, e.g. a vector-field. Mathematically, 'structured' domains are manifolds and 'unstructured' domains are fibers/Fasern that are defined on top of the product of all manifolds. Together, everything forms a fiber-bundle.
See, for reference:
https://en.wikipedia.org/wiki/Fiber_bundle2018-04-04https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/34Add fft_machine keyword to the rg_space.copy()2018-04-02T09:42:38ZTheo SteiningerAdd fft_machine keyword to the rg_space.copy()https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/43field.smooth on rg_space produces non-sensible results on multiple spaces2018-04-02T09:42:38ZTheo Steiningerfield.smooth on rg_space produces non-sensible results on multiple spaces from nifty import *
x = rg_space((8,))
f = field((x,x), val=1)
f.val[3] = 10
f.smooth(spaces=0, sigma=0.0001).val
The result of the last operation does not make sense, as the columns are
<distributed_data_object>
array([ 1.00000104, 0.99999867, 1.00000355, 5.49999423, 1.00000607,
5.49999423, 1.00000355, 0.99999867])
instead of
<distributed_data_object>
array([ 1.00000104, 0.99999822, 1.00000607, 9.99999023, 1.00000607,
0.99999822, 1.00000104, 0.99999911])
from nifty import *
x = rg_space((8,))
f = field((x,x), val=1)
f.val[3] = 10
f.smooth(spaces=0, sigma=0.0001).val
The result of the last operation does not make sense, as the columns are
<distributed_data_object>
array([ 1.00000104, 0.99999867, 1.00000355, 5.49999423, 1.00000607,
5.49999423, 1.00000355, 0.99999867])
instead of
<distributed_data_object>
array([ 1.00000104, 0.99999822, 1.00000607, 9.99999023, 1.00000607,
0.99999822, 1.00000104, 0.99999911])
https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/45Possibly invert Fourier-transformation responsibilities2018-04-02T09:42:38ZTheo SteiningerPossibly invert Fourier-transformation responsibilitiesRight now, the domain-space computes the data for the codomain-space. The field then uses this data together with the new data. For the spherical spaces this produces strange if-else blocks in the calc_transform methods.
It would be clearer when the codomain-space computed its data directly: e.g. lm_space then has two sub-methods: transform_from_hp and transform_from_glRight now, the domain-space computes the data for the codomain-space. The field then uses this data together with the new data. For the spherical spaces this produces strange if-else blocks in the calc_transform methods.
It would be clearer when the codomain-space computed its data directly: e.g. lm_space then has two sub-methods: transform_from_hp and transform_from_glhttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/49Move `check_codomain` in Transformation to child classes2018-04-02T09:42:38ZTheo SteiningerMove `check_codomain` in Transformation to child classeshttps://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/50Use gdi(gc[fft_module]) in RGRGTransformation.__init__2018-04-02T09:42:38ZTheo SteiningerUse gdi(gc[fft_module]) in RGRGTransformation.__init__https://gitlab.mpcdf.mpg.de/ift/nifty/-/issues/52Merge Transform and Transformation classes2018-04-02T09:42:38ZTheo SteiningerMerge Transform and Transformation classes