Skip to content
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

Adjust documentation after review #2175

Merged
merged 37 commits into from
Sep 23, 2020

Conversation

JanuszL
Copy link
Contributor

@JanuszL JanuszL commented Aug 3, 2020

Signed-off-by: Janusz Lisiecki jlisiecki@nvidia.com

Why we need this PR?

Pick one, remove the rest

  • It fixes a numerous language and formatting defects in the documentation

What happened in this PR?

Fill relevant points, put NA otherwise. Replace anything inside []

  • What solution was applied:
    fixes a numerous language and formatting defects in the documentation
  • Affected modules and functionalities:
    whole documentation
  • Key points relevant for the review:
    NA
  • Validation and testing:
    CI build and documentation review of the output
  • Documentation (including examples):
    Documentation has been updated

JIRA TASK: [DALI-1552]

@JanuszL JanuszL force-pushed the update_docs_after_review branch 12 times, most recently from 6000793 to 3d4c9ec Compare August 11, 2020 10:12
@JanuszL
Copy link
Contributor Author

JanuszL commented Aug 12, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1538926]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1538926]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1538926]: BUILD PASSED

@JanuszL JanuszL force-pushed the update_docs_after_review branch 3 times, most recently from 3cc1b0a to 1dd7b04 Compare September 4, 2020 08:32
.DocStr(R"code(Converts a Spectrogram to a mel Spectrogram using triangular filter banks.
Expects an input with 2 or more dimensions where the last two dimensions correspond to the
fft bin index and the window index respectively.)code")
.DocStr(R"code(Converts a Spectrogram to a mel spectrogram by using triangular filter banks.
Copy link
Contributor

@mzient mzient Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.DocStr(R"code(Converts a Spectrogram to a mel spectrogram by using triangular filter banks.
.DocStr(R"code(Converts a spectrogram to a mel spectrogram by applying a bank of triangular filters.
  1. Banks are not triangular. Filters are.
  2. It converts a spectrogram (data), not the operator.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 48 to 49
R"code(Determines whether to normalize the triangular filter weights by the width
of their mel bands.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
R"code(Determines whether to normalize the triangular filter weights by the width
of their mel bands.
R"code(Determines whether to normalize the triangular filter weights by the width
of their frequency bands.

The weights are normalized in input domain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

consistent with the implementation of the Hidden Markov Toolkit (HTK).
)code",
"slaney");
R"code(Determines the formula that will be used to convert frequencies from Hertz to mel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
R"code(Determines the formula that will be used to convert frequencies from Hertz to mel
R"code(Determines the formula that will be used to convert frequencies from hertz to mel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

)code",
"slaney");
R"code(Determines the formula that will be used to convert frequencies from Hertz to mel
and from mel to Hertz.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
and from mel to Hertz.
and from mel to hertz.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

R"code(Determines the formula that will be used to convert frequencies from Hertz to mel
and from mel to Hertz.

The mel scale is a perceptual scale of pitches, so there is no formula.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We select a formula and say there's not formula? The previous variant was much better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

8192);
Unless specified otherwise, ``reference_power`` is typically the maximum of the signal.

Here is a list of the inputs and outputs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Here is a list of the inputs and outputs:
Inputs and outputs:

Hopefully we can insert an empty line. I think the original header was better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

.NumInput(1)
.NumOutput(detail::kNumOutputs)
.AddOptionalArg("cutoff_db",
R"code(The threshold [dB], below which everything is considered as silence.)code",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
R"code(The threshold [dB], below which everything is considered as silence.)code",
R"code(The threshold, in dB, below which the signal is considered silent.)code",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 45 to 47
.AddOptionalArg("window_length", R"code(Size of a sliding window.

This window is used to calculate the short-term power of the signal.)code", 2048)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.AddOptionalArg("window_length", R"code(Size of a sliding window.
This window is used to calculate the short-term power of the signal.)code", 2048)
.AddOptionalArg("window_length", R"code(Size of the sliding window used in the calculation of
the short-term power of the signal.)code", 2048)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


This window is used to calculate the short-term power of the signal.)code", 2048)
.AddOptionalArg("reference_power",
R"code(The reference power that is used to convert the signal to db.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
R"code(The reference power that is used to convert the signal to db.
R"code(The reference power that is used to convert the signal to dB.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment on lines 51 to 52
If a value for reference_power is not provided, the maximum value of the signal will be used as the
reference power.)code",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
If a value for reference_power is not provided, the maximum value of the signal will be used as the
reference power.)code",
If a value for reference_power is not provided, the maximum power of the signal will be used as the
reference.)code",

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1641630]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1641630]: BUILD FAILED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL
Copy link
Contributor Author

JanuszL commented Sep 21, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1641646]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1641646]: BUILD PASSED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL JanuszL force-pushed the update_docs_after_review branch 4 times, most recently from 2237b9a to d1a6c28 Compare September 23, 2020 08:50
@JanuszL
Copy link
Contributor Author

JanuszL commented Sep 23, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1647716]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1647716]: BUILD FAILED

@JanuszL
Copy link
Contributor Author

JanuszL commented Sep 23, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1647874]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1647874]: BUILD FAILED

Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>
@JanuszL
Copy link
Contributor Author

JanuszL commented Sep 23, 2020

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1648152]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1648152]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [1648152]: BUILD PASSED

@JanuszL JanuszL merged commit e401165 into NVIDIA:master Sep 23, 2020
@JanuszL JanuszL deleted the update_docs_after_review branch September 23, 2020 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants