Skip to content

Remove FunctionSpace and Discretization#1459

Merged
kohr-h merged 9 commits intoodlgroup:masterfrom
kohr-h:remove_fspace
Mar 31, 2020
Merged

Remove FunctionSpace and Discretization#1459
kohr-h merged 9 commits intoodlgroup:masterfrom
kohr-h:remove_fspace

Conversation

@kohr-h
Copy link
Copy Markdown
Member

@kohr-h kohr-h commented Feb 5, 2019 •
edited
Loading

Rough Overview:

  • Discretization and DiscretizationElement are gone, functionality has been moved onto DiscreteLp and DiscreteLpElement

  • FunctionSpace and FunctionSpaceElement are gone, functionality for vectorization and evaluation has been broken into a bunch of functions that live in odl.discr.discr_utils.
    For instance, FunctionSpace.element() created a wrapper that unified the interface of functions with and without out argument. This is done in the function make_func_for_sampling.

  • Support for non-vectorized functions in DiscreteLp.element() is removed.
    Rationale: Why did we ever support this?

  • Some parts of discretization.py (now gone) are inlined in lp_discr.py, which makes it appear as if it had changed a lot. It hasn't.

  • discr_sequence_space is gone.
    Rationale: lack of usefulness.

  • FunctionSpaceMapping classes have simply been turned into functions and moved over to the discr_utils module done in API: move 'interp' out of spaces #1520

  • interp is no longer a property of the space but up to each operator (which I consider to be the better approach) done in API: move 'interp' out of spaces #1520

Closes #1457

@pep8speaks
Copy link
Copy Markdown

pep8speaks commented Feb 5, 2019 •
edited
Loading

Checking updated PR...

Line 1568:80: E501 line too long (88 > 79 characters)
Line 1468:80: E501 line too long (88 > 79 characters)

Line 1083:80: E501 line too long (88 > 79 characters)
Line 1050:1: W293 blank line contains whitespace

Comment last updated at 2020-03-31 13:29:10 UTC

@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Feb 5, 2019

Maybe it would already now be good to have some input. From the point of view of functionality, this PR is complete. A big part of it is just moving code around and wiring it up again, so I don't think it's that much to review after all.

@kohr-h kohr-h force-pushed the remove_fspace branch 2 times, most recently from b608ad5 to a090094 Compare February 7, 2019 22:45
@kohr-h kohr-h changed the title WIP: Remove FunctionSpace, Discretization and FunctionSpaceMapping Remove FunctionSpace, Discretization and FunctionSpaceMapping Feb 7, 2019
@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Feb 8, 2019

All tests are fixed now. I haven't had the opportunity to check ASTRA CUDA yet, but there were no changes, so it should work. Will look at it soon.

Anyway, now the PR is definitely ripe for review.

@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Feb 8, 2019

I'll also look into the PEP8 issues.

@kohr-h kohr-h force-pushed the remove_fspace branch 2 times, most recently from c4b4592 to fc49909 Compare February 10, 2019 10:24
@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Feb 18, 2019

Any chance to get a review on this PR? I know it looks horribly large from the numbers, but a lot is due to movement of code. When classes go away that were contained in their own modules, it's unavoidable that their functionality move to other files. Thus you get these inflated numbers.

If it helps, I can write a small summary of how things used to be wired up and what it looks like now, so it becomes easier to digest and review.

@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Feb 18, 2019

I'll also rebase and get rid of the PEP8 warnings.

@adler-j
Copy link
Copy Markdown
Member

adler-j commented Feb 18, 2019

Please try to fix the conflicts etc, then I can review this later (although perhaps not this week, it's chaos).

@kohr-h kohr-h mentioned this pull request Feb 20, 2019
Draft
@kohr-h kohr-h force-pushed the remove_fspace branch 2 times, most recently from f4ff6f5 to 4d7e59d Compare March 13, 2019 21:18
kohr-h
kohr-h commented Apr 15, 2019
View reviewed changes
Comment thread odl/discr/discr_utils.py
kohr-h
kohr-h commented Apr 15, 2019
View reviewed changes
Comment thread odl/util/utility.py Outdated
@kohr-h kohr-h mentioned this pull request Apr 16, 2019
Merged
@kohr-h kohr-h mentioned this pull request May 20, 2019
Open
@kohr-h kohr-h mentioned this pull request Aug 14, 2019
Merged
@kohr-h kohr-h force-pushed the remove_fspace branch 3 times, most recently from 86fc68e to b017445 Compare January 25, 2020 00:16
@kohr-h kohr-h changed the title Remove FunctionSpace, Discretization and FunctionSpaceMapping Remove FunctionSpace and Discretization Jan 25, 2020
kohr-h
kohr-h commented Jan 26, 2020
View reviewed changes
Comment thread odl/discr/discr_ops.py
kohr-h
kohr-h commented Jan 26, 2020
View reviewed changes
Comment thread odl/discr/discr_utils.py Outdated
kohr-h
kohr-h commented Mar 31, 2020
View reviewed changes
Comment thread odl/discr/lp_discr.py Outdated
kohr-h
kohr-h commented Mar 31, 2020
View reviewed changes
Comment thread odl/discr/lp_discr.py Outdated
kohr-h
kohr-h commented Mar 31, 2020
View reviewed changes
Comment thread odl/discr/lp_discr.py Outdated
kohr-h
kohr-h commented Mar 31, 2020
View reviewed changes
Comment thread odl/test/discr/lp_discr_test.py Outdated
kohr-h
kohr-h commented Mar 31, 2020
View reviewed changes
Comment thread odl/test/discr/lp_discr_test.py Outdated
kohr-h
kohr-h commented Mar 31, 2020
View reviewed changes
Comment thread odl/trafos/fourier.py Outdated
kohr-h
kohr-h commented Mar 31, 2020
View reviewed changes
Comment thread odl/trafos/wavelet.py Outdated
kohr-h added 8 commits March 31, 2020 12:46
@kohr-h
API: Remove Discretization and FunctionSpace
04e2517
@kohr-h
Improve fn signature match with docstrings, plus other fixes
68e7741 
- Fix signatures/docstrings: explicit function parameters should
  be mentioned under "Parameters" in the docstring, and implicit
  ones in `kwargs` should be under "Other Parameters".
- Fix issue in `ResizingOperator.adjoint`, where `pad_mode` is
  looked up wrongly.
- Fix missing parameters in error string formatting
- Implement missing abstract methods `default_dtype` and
  `available_dtypes` in `DiscreteLp`
- Use previously ignored `dtype` parameter in test of
  `ResizingOperator`
- Correctly handle failure in entry part of context managers
- Fix bad test for ValueError with wrong argument
- Remove some unused variables
- Rename functions for mapping PyFFTW flags to ODL convention and back
- Simplify boolean expressions
- Remove unused `with_metaclass` function
- Correct spelling errors
@kohr-h
Small fixes in docs and comments
fe2bd74
@kohr-h
Rename DiscreteLp->DiscretizedSpace, and other fixes
060411a 
- Rename DiscreteLp and DiscreteLpElement to DiscretizedSpace and
  DiscretizedSpaceElement
- Rename make_func_for_sampling to sampling_function
- Infer output shape of vector-valued functions from array shape
- Shorten About ODL intro
- Inline confusing _all_interp_equal function
- Inline getargspec
- Rename none_context to nullcontext
@kohr-h
Remove CUDA line from README
0609c70
@kohr-h
Apply suggestions from code review
54be65b
@kohr-h
PEP8 fixes
d4f4518
@kohr-h
More PEP8 fixes and update of odl/README.md
5ae37e5
@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Mar 31, 2020

All comments addressed and rebased on master. Unless there are complaints (by a reviewer or by CI) I'll merge later today.

@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Mar 31, 2020

One last thing I did was to rename the lp_discr module to discr_space, in line with the rename of DiscreteLp. I'll wait for CI and then merge.

@kohr-h kohr-h merged commit 5d56b10 into odlgroup:master Mar 31, 2020
@kohr-h kohr-h deleted the remove_fspace branch March 31, 2020 14:18
@adler-j
Copy link
Copy Markdown
Member

adler-j commented Apr 3, 2020

Very well done on getting this in Holger!

@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Apr 3, 2020

My pleasure. Working on #1475 again, and on #1540 with Adriaan.

@kohr-h
Copy link
Copy Markdown
Member Author

kohr-h commented Apr 3, 2020

And thanks to Axel for reviewing!

@kohr-h kohr-h mentioned this pull request Apr 13, 2020
Merged
@kohr-h kohr-h mentioned this pull request May 8, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adler-j adler-j adler-j left review comments

@aringh aringh aringh approved these changes

Assignees

No one assigned

Labels

area: core type: api change type: complexity

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove FunctionSpace, Discretization and FunctionSpaceMapping classes

4 participants

@kohr-h @pep8speaks @adler-j @aringh