-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doc update working example #42
Conversation
…ork flow Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: Travis F. Collins <[email protected]>
…dings Add doc for python bindings
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
doc/working_example.md
Outdated
<summary>Code Snippet</summary> | ||
|
||
``` Python | ||
def main(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove use of the main function abstraction. It just makes the examples confusing and wastes space
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
…cript Signed-off-by: spagadar <[email protected]>
myst is complaining about cross references. WARNING: 'myst' cross-reference target not found Need to add this to the conf.py # -- General configuration ---------------------------------------------------
# Add any Sphinx extension module names here, as strings. They can be
# extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
# ones.
extensions = ["breathe", "sphinx.ext.autodoc", "myst_parser", "sphinx_inline_tabs", "sphinx.ext.graphviz"]
+# Fix use of autogenerated cross-reference warning
+myst_heading_anchors = 2
# Add any paths that contain templates here, relative to this directory.
templates_path = ["_templates"]
|
…c generation Signed-off-by: spagadar <[email protected]>
…acts Signed-off-by: spagadar <[email protected]>
…acts Signed-off-by: spagadar <[email protected]>
…tifacts for CoverageTest Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
…artifacts for CoverageTest Signed-off-by: spagadar <[email protected]>
Updated conf.py. |
import pprint | ||
import matplotlib.pyplot as pl | ||
from matplotlib.patches import Rectangle as MPRect | ||
from tabulate import tabulate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This package needs to be added to the requirements_doc.txt
bindings/python/foo.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filename should be descriptive
doc/python/genalyzer.simplified.rst
Outdated
@@ -0,0 +1,7 @@ | |||
Simplified API | |||
============ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heading directive "======" too short. Needs to be as long as text
doc/working_example.md
Outdated
@@ -0,0 +1,825 @@ | |||
# Genalyzer Through Examples | |||
## A Minimal Working Example | |||
The workflow of Genalyzer is best understood with the help of a working example. We use Python to illustrate this workflow in the current section. The data that we will process using Genalyzer for this example is also generated using Genalyzer. It consists of a ``300 KHz`` tone sampled at ``3 MSPS``. The only source of noise in this synthetic data is quantization noise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use Python to illustrate this workflow in the current section. The data that we will process using Genalyzer for this example is also generated using Genalyzer. It consists of a
300 KHz
tone sampled at3 MSPS
. The only source of noise in this synthetic data is quantization noise.
This doesn't flow very well. Can you simplify it a bit?
doc/working_example.md
Outdated
<br/><br/> | ||
|
||
### Compute FFT | ||
We begin with the initial steps involved, _i.e.,_, configuring Genalyzer and performing FFT on the generated data. Since spectral analysis is the core of Genalyzer, computing FFT is the first important step. Expand the code-snippet below for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expand the code-snippet below for details.
Remove the text and keep the code expanded by default
doc/working_example.md
Outdated
|
||
We will expand this code snippet to illustrate the various capabilities of Genalyzer. We postpone the discussion on how to generate synthetic data using Genalyzer and the related knobs available. We begin first with the FFT-computing step in the above snippet. | ||
|
||
The arguments that ``fft()`` takes are shown on [here](#genalyzer.advanced.advanced.fft). As shown on this page, it takes a variable number of arguments to handle several usecases. The usecases are split based on how the I/Q samples are represented, _i.e._, whether the samples are complex-valued, or interleaved I/Q, or split into I and Q separately. These categories are further split based on whether the samples are represented as floating-point or fixed-point integer data. In the latter case, the resolution in bits and code-format are the required inputs. The supported datatypes for the input data vector are: ``complex128``, ``float64``, ``int16``, ``int32``, ``int64``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refrain from using "i.e." a lot. It can be distracting.
I don't really understand the point of this paragraph. Are you trying to explain how to use the fft function? its flexibility? or its nuanced API?
For individual arguments just refer users to the reference doc. This shouldn't be a copy+paste of that here.
This section is "Compute FFT" but it probably needs to be more specific on the data types used since that's what drives everything.
doc/working_example.md
Outdated
</details> | ||
|
||
##### Create ``fourier_analysis`` Object | ||
The first step in configuring Genalyzer is to call the ``fa_create()`` function with an _object\_key_. This key is used for all configuring Genalyzer further, and for computing and retrieving the metrics. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't we on like step 2 at this point?
This key is used for all configuring Genalyzer further
I think you meant configuration and not configuring
Can we use some other name than "object_key"? Is there something more relatable we can use?
doc/working_example.md
Outdated
```{admonition} Aside | ||
:class: note | ||
|
||
Under the hood, Genalyzer adds a key-value pair to a ``static`` ``map`` container to manage the metrics to be computed. The key is the string argument passed through ``fa_create()``, and the mapped value is a shared-pointer to an instance of ``fourier_analysis`` class. This key is then used to further configure Genalyzer, and to compute and retrieve the metrics through ``fourier_analysis`` class. The intent behind using a ``map`` container is to be easily able to associate multiple keys to different snapshots of the data being analyzed and to have the metrics for each of those snapshots available. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to inspect it?
doc/working_example.md
Outdated
<br/><br/> | ||
|
||
##### Identify Signal Tone | ||
The next step is to identify the signal tone and label it. This is done by calling either [``fa_fixed_tone()``](#genalyzer.advanced.advanced.fa_fixed_tone) or [``fa_max_tone()``](#genalyzer.advanced.advanced.fa_max_tone) functions. In both cases, the first two arguments are _keys_. The first _key_ is the _object\_key_ mentioned previously, and the second _key_ is a string that is to be associated with the signal component whose magnitude is measured and used subsequently when computing performance metrics. In both cases, the third argument is a tag which maps to the component type. To identify a component as the signal tone, we use ``GnFACompTagSignal``. The other tags available are shown [here](#genalyzer.advanced.advanced.FaCompTag). As the name indicates, ``fa_max_tone()`` interprets the tone with the highest magnitude as the signal tone. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next step is to identify the signal tone and label it. This is done by calling either
fa_fixed_tone()
orfa_max_tone()
functions. In both cases, the first two arguments are keys. The first key is the object_key mentioned previously, and the second key is a string that is to be associated with the signal component whose magnitude is measured and used subsequently when computing performance metrics. In both cases, the third argument is a tag which maps to the component type. To identify a component as the signal tone, we useGnFACompTagSignal
. The other tags available are shown here. As the name indicates,fa_max_tone()
interprets the tone with the highest magnitude as the signal to
The descriptions of keys and tags here is a bit confusing. Is there a plain way to describe what they are for?
doc/working_example.md
Outdated
The next step is to identify the signal tone and label it. This is done by calling either [``fa_fixed_tone()``](#genalyzer.advanced.advanced.fa_fixed_tone) or [``fa_max_tone()``](#genalyzer.advanced.advanced.fa_max_tone) functions. In both cases, the first two arguments are _keys_. The first _key_ is the _object\_key_ mentioned previously, and the second _key_ is a string that is to be associated with the signal component whose magnitude is measured and used subsequently when computing performance metrics. In both cases, the third argument is a tag which maps to the component type. To identify a component as the signal tone, we use ``GnFACompTagSignal``. The other tags available are shown [here](#genalyzer.advanced.advanced.FaCompTag). As the name indicates, ``fa_max_tone()`` interprets the tone with the highest magnitude as the signal tone. | ||
|
||
```{note} | ||
Since ``fa_fixed_tone()`` requires that the frequency of a desired tone is provided as an argument, it is useful for taking into account any desired tone that is pertinent to analysis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is useful for taking into account any desired tone that is pertinent to analysis.
I think its more than useful, its actually required in this case right?
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
bindings/python/fft.png
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Image files should not be in source code folders. They need to be located with doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
doc/spectral_analysis.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is no real code in the page. Am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pointing readers to files in this manner:
genalyzer/doc/spectral_analysis.md
Line 164 in 31b210b
In [this](../../../bindings/python/examples/gn_doc_spectral_analysis1.py) Python script, FFT analysis is run by the following line: |
There are small code blocks also:
https://github.com/analogdevicesinc/genalyzer/blob/50cd0f1eed1e1bf2951097406ecf2520cbbba578/doc/spectral_analysis.md?plain=1#L123C1-L123C23
genalyzer/doc/spectral_analysis.md
Line 136 in 50cd0f1
```{code-block} python |
genalyzer/doc/spectral_analysis.md
Line 165 in 50cd0f1
```{code-block} python |
genalyzer/doc/spectral_analysis.md
Line 201 in 50cd0f1
```{code-block} python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really hard to follow. Code snippets should be in the doc so people don't have to keep context switching
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
…s to provide better context Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
Signed-off-by: spagadar <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is in good shape. Can you clean up the git history and we'll merge
No description provided.