Repository navigation
feat: validate the complex channel axis; fix the documentation build - #32
Merged
Merged
Conversation
ruff 0.17 joins implicitly concatenated strings that fit on one line, so "ruff format --check" fails on complexnn/conv.py as it is on main. Apply that change alone, to the three copies of the "channel dimension should be defined" message, so the next commit only holds the channel axis validation. No behaviour change.
The layers store complex values as real tensors: the first half of the channel axis holds the real parts and the second half the imaginary parts. An odd or single channel used to fail with errors that did not say so: an input spec asking for size 0 (#15), a bare AssertionError in ComplexDense, or a shape mismatch deep inside ComplexBatchNormalization and ComplexLayerNorm. GetReal, GetImag and GetAbs returned halves of different sizes without complaint. Add _check_complex_axis to utils.py and call it from the build methods of ComplexConv (1D, 2D, 3D and transposed), ComplexDense, ComplexBatchNormalization, ComplexLayerNorm, GetReal, GetImag and GetAbs. It raises a ValueError that names the layer, the axis and the received shape, and explains the layout. The convolution message also says which data_format chose the axis, which is what caught out the reporter of #15. The normalisation layers point to their axis argument. Every rejected shape already failed on Keras 2 and Keras 3, except GetReal, GetImag and GetAbs on an odd size. Those three skip the check when the size is undefined, because the dynamic split in call handles that case on Keras 2. Tests cover odd and too small sizes for each layer, the exact layer from #15, and the smallest valid layout. Refs #15
The latest Read the Docs build (June 2024) failed with "No module named 'recommonmark'": conf.py loads recommonmark, but docs/requirements.txt never listed it, nor sphinx-rtd-theme. Even with both installed, autodoc cannot import complexnn because the package is not installed, so the live site still serves a 2022 build with empty API pages. - docs/requirements.txt: use myst-parser, which replaces the deprecated recommonmark, add sphinx-rtd-theme, which conf.py sets as html_theme, and drop tensorflow, which now comes in as a dependency of the package itself. - conf.py: load myst_parser instead of recommonmark and drop html_static_path, which points at a _static directory that does not exist and makes Sphinx warn. - .readthedocs.yml: move from ubuntu-20.04, which is end of life, to ubuntu-24.04 with Python 3.12, and install the package before the docs requirements so autodoc can import it. - .gitignore: ignore docs/build and the API pages that sphinxcontrib-apidoc writes to docs/source/api on every build.
Two warnings that "sphinx-build -W" turns into errors: - myst-parser expects a document to start with a level 1 heading, and cite.md started at level 2. - license.rst was not in any toctree, so no page linked to it. Add it to the table of contents after the citation page.
The docstrings used the Keras 2 convention of "# Arguments" headings followed by an indented block. docutils reads those as a paragraph and an unexpected indent, which gave 40 errors and warnings in the API pages under "sphinx-build -W". Turn each "# Section" heading into "Section:" after a blank line, and enable sphinx.ext.napoleon, which renders Arguments, Returns, Raises and References as field lists. "Input shape" and "Output shape" are registered as custom sections. Only the heading lines change; the text of each section stays as it was. Also set autodoc_inherit_docstrings to False. Methods without their own docstring otherwise show the one from the Keras base class, which differs between Keras versions, and the one WeightNorm_Conv.call inherits from TensorFlow's private Keras copy is not valid reStructuredText.
The layers expect complex inputs as real tensors with the real parts in the first half of the channel axis and the imaginary parts in the second half, and they return outputs in the same layout. Little of this was written down: the README showed a 3x3 example with the parts as two stacked blocks, and the docs it linked to did not describe the layout at all. The reporter of #15 had put the parts on an axis of their own. - docs: add a "Data layout" page with an example, a NumPy recipe for both directions, the axis each layer uses and what the new ValueError means. - README: say in Usage what input_shape=(28, 28, 2) means and rewrite "Complex Format of Tensors" to match the code, linking the new page. - Docstrings: replace the odd channel counts in the ComplexConv2D and ComplexConv3D examples, which the layers now reject, note the layout in the conv and dense input and output shapes, correct the ComplexDense output shape to 2 x units, and correct the ComplexBatchNormalization hint for channels_first data from axis=2 to axis=1. Refs #15
Build inputs with the NumPy recipe from the README, real parts first, and check that: - GetReal, GetImag and GetAbs return the real parts, the imaginary parts and the magnitudes; - a 1x1 ComplexConv2D multiplies each pixel by the complex matrix held in its kernel; - ComplexConv1D, ComplexConv2D, ComplexConv3D, transposed ComplexConv2D and ComplexDense are complex linear: multiplying the inputs by i multiplies the outputs by i, which only holds when inputs and outputs both use the documented layout. With an interleaved layout instead, every one of these checks fails.
The Read the Docs build has failed since June 2024, and no check on GitHub showed it, because nothing else built the documentation. Add a docs job that installs the package and docs/requirements.txt, as .readthedocs.yml does, and runs sphinx-build with -W, so a change that breaks the docs or adds a Sphinx warning fails on the pull request. --keep-going reports every warning in one run. fetch-depth 0 gives setuptools-scm the tags it needs to compute the version.
The table grouped ComplexConv1D with ComplexConv2D and ComplexConv3D and said it uses axis 1 with data_format="channels_first" and follows the Keras image data format otherwise. Neither holds: ComplexConv1D has no data_format argument (passing one raises a TypeError) and always passes data_format="channels_last" to ComplexConv, so its complex axis is always the last axis. Give ComplexConv1D a row of its own, and say that the channels_first recipe in "Converting complex data" applies to ComplexConv2D and ComplexConv3D only.
…ormat
The axis table said that ComplexConv2D and ComplexConv3D use the Keras
image data format when data_format is not given, the same phrase it
used for GetReal, GetImag and GetAbs. The two do not read the same
setting. ComplexConv resolves data_format=None with conv_utils from
TensorFlow's internal Keras copy, which reads keras.json at import but
not a later keras.backend.set_image_data_format() call. GetReal,
GetImag and GetAbs call tensorflow.keras.backend.image_data_format()
and do follow that call. On Keras 3, Keras 2 and tf-keras alike, after
set_image_data_format("channels_first") a default ComplexConv2D still
uses channels_last.
Say so in the table, and advise passing data_format to the
convolutions explicitly when the format is changed at runtime. The code
is unchanged: making the convolutions follow the runtime setting is a
behaviour change for a follow-up.
The new layout error for ComplexBatchNormalization and ComplexLayerNorm told users to "use the `axis` argument to choose the axis", and the axis table said they use whatever axis that argument names. call() only handles two settings: axis=-1, and axis=1 for inputs of rank 2, 4 or 5. Any other value either raises "axis should be either 1 or -1" or splits the last axis anyway. With axis=1 on a rank 3 input, the build check passes and call() then fails with an opaque InvalidArgumentError. Name the two supported settings in the error hint, in the axis table and in the ComplexBatchNormalization docstring. Test the hint for both layers, and test that every documented setting runs on inputs of rank 2 to 5. Rejecting the unsupported combinations at build time would change behaviour, so it is left for a follow-up.
The rewritten "Complex Format of Tensors" section linked to /en/latest/layout.html and /en/latest/math.html. keras-complex on Read the Docs is a single-version project that serves its pages at the root, so both links return 404, while /math.html, the link the README used before, returns 200. The layout page will also be served at the root once Read the Docs builds it. Use the root URLs, as the badge does, and keep the #implementation anchor the old math link had.
Every "too small" case used size 1, which the odd-size condition already rejects, so no test reached the separate "at least 2" condition: without it, tests/test_validation.py still passed on Keras 3 and Keras 2. The condition matters. On main, a complex axis of size 0 built without an error for: - ComplexConv1D, ComplexConv2D and ComplexConv3D on Keras 3; - transposed ComplexConv2D on Keras 3, Keras 2 and tf-keras; - ComplexBatchNormalization and ComplexLayerNorm; - GetReal, GetImag and GetAbs. ComplexDense raised a ZeroDivisionError. Add size 0 cases for every validated layer, and size 1 cases for transposed convolution and GetReal, GetImag and GetAbs, which only had odd sizes of 3 and 5. With the "at least 2" condition removed, six test methods now fail on both Keras versions, eight failures counting subtests.
These layers have no weights, so one instance is often reused, but Keras builds a layer once and the layout check only ran in build. It now also runs in call, which covers eager tensors and symbolic tensors on Keras 2, and in compute_output_shape, which Keras 3 uses for symbolic tensors.
# Conflicts: # complexnn/conv.py # complexnn/dense.py # complexnn/norm.py # complexnn/utils.py
…re created Since the move to complexnn's own conv_utils, ComplexConv2D and ComplexConv3D take data_format=None from keras.backend.image_data_format() when they are created, instead of reading only keras.json. GetReal, GetImag and GetAbs read it on every call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stage 2, PR 3 of 4. Clear errors for an invalid complex axis (closes #15), documentation of the data layout, and a working documentation build.
What changes
ValueErrorthat explains the layout. Before, they failed somewhere inside the computation or quietly produced wrong shapes.call, and incompute_output_shapefor Keras 3 symbolic tensors. Before, only the first input was checked.myst-parserinstead ofrecommonmark.docsjob builds withsphinx-build -W.Verification
input_shapeones)sphinx-build -W --keep-goingas in CIruff format --check, pre-commit and the flake8 critical check are clean.Merge notes
faf1a04):_ComplexPartsubclasses;5d21469correctsdocs/source/layout.md. Since refactor: public Keras APIs only, no input_shape warnings #30 the convolutions takedata_format=Nonefrom the image data format when they are created, not only fromkeras.json.-Wpasses.