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.

Sign up for GitHub

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

Jump to bottom

Documentation fixes and improvements #2289

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Documentation fixes and improvements #2289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1237fb9
Add link the module documentation for Linear.
iwanders Jun 25, 2024
175a52e
Fix warnings to undefined members, add loss module summary.
iwanders Jun 25, 2024
6773de8
Fix documentation warnings in the quantized module.
iwanders Jun 25, 2024
cb90310
Update unsafe reason with correct link in documentation.
iwanders Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion candle-core/src/quantized/ggml_file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ fn from_raw_data<T: super::GgmlType + Send + Sync + 'static>(
super::QTensor::new(data, dims)
}

/// Creates a [Tensor] from a raw GGML tensor.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

137 | /// Creates a [Tensor] from a raw GGML tensor.
    |                ^^^^^^ no item named `Tensor` in scope
    |
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
    = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

/// Creates a [`crate::Tensor`] from a raw GGML tensor.
pub fn qtensor_from_ggml(
ggml_dtype: GgmlDType,
raw_data: &[u8],
Expand Down
2 changes: 1 addition & 1 deletion candle-core/src/quantized/gguf_file.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Support for the GGUF file format.
//!
//! Spec: https://github.com/philpax/ggml/blob/gguf-spec/docs/gguf.md
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  |
3 | //! Spec: https://github.com/philpax/ggml/blob/gguf-spec/docs/gguf.md
  |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://github.com/philpax/ggml/blob/gguf-spec/docs/gguf.md>`
  |
  = note: bare URLs are not automatically turned into clickable links
  = note: `#[warn(rustdoc::bare_urls)]` on by default

//! Spec: <https://github.com/philpax/ggml/blob/gguf-spec/docs/gguf.md>

use super::{GgmlDType, QTensor};
use crate::{Device, Result};
Expand Down
2 changes: 2 additions & 0 deletions candle-nn/src/linear.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
//! ```
use candle::{Result, Tensor};

/// See the documentation for [`mod@super::linear`] for more information.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rendered:

image

I didn't want to duplicate the module docs, it's not great, but it helps people find the actual docs.

#[derive(Clone, Debug)]
pub struct Linear {
weight: Tensor,
Expand Down Expand Up @@ -76,6 +77,7 @@ pub fn linear_no_bias(in_dim: usize, out_dim: usize, vb: crate::VarBuilder) -> R
Ok(Linear::new(ws, None))
}

/// Create or initialize a new linear layer, bias depending on the argument.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function was the only one in the module without a line of documentation. THe function just switches between the other two based on the bias boolean, so I just added this.

pub fn linear_b(
in_dim: usize,
out_dim: usize,
Expand Down
13 changes: 7 additions & 6 deletions candle-nn/src/loss.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
//! Loss functions.
use candle::{Result, Tensor};

/// The negative log likelihood loss.
///
/// Arguments
///
/// * [inp]: The input tensor of dimensions `N, C` where `N` is the batch size and `C` the number
/// * `inp`: The input tensor of dimensions `N, C` where `N` is the batch size and `C` the number
/// of categories. This is expected to contain log probabilities.
/// * [target]: The ground truth labels as a tensor of u32 of dimension `N`.
/// * `target`: The ground truth labels as a tensor of u32 of dimension `N`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were all unresolved links:

warning: unresolved link to `target`
  --> /home/ivor/Documents/Code/rust/playground_candle/candle/candle-nn/src/loss.rs:36:8
   |
36 | /// * [target]: The ground truth labels as a tensor of u32 of dimension `N`.
   |        ^^^^^^ no item named `target` in scope
   |
   = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

There is no current official way to document function arguments, I followed rust-lang/rust#57525.

Rendered:

image

///
/// The resulting tensor is a scalar containing the average value over the batch.
pub fn nll(inp: &Tensor, target: &Tensor) -> Result<Tensor> {
Expand All @@ -31,9 +32,9 @@ pub fn nll(inp: &Tensor, target: &Tensor) -> Result<Tensor> {
///
/// Arguments
///
/// * [inp]: The input tensor of dimensions `N, C` where `N` is the batch size and `C` the number
/// * `inp`: The input tensor of dimensions `N, C` where `N` is the batch size and `C` the number
/// of categories. This is expected to raw logits.
/// * [target]: The ground truth labels as a tensor of u32 of dimension `N`.
/// * `target`: The ground truth labels as a tensor of u32 of dimension `N`.
///
/// The resulting tensor is a scalar containing the average value over the batch.
pub fn cross_entropy(inp: &Tensor, target: &Tensor) -> Result<Tensor> {
Expand All @@ -53,9 +54,9 @@ pub fn mse(inp: &Tensor, target: &Tensor) -> Result<Tensor> {
///
/// Arguments
///
/// * [inp]: The input tensor of dimensions `N, C` where `N` is the batch size and `C` the number
/// * `inp`: The input tensor of dimensions `N, C` where `N` is the batch size and `C` the number
/// of categories. This is expected to raw logits.
/// * [target]: The ground truth labels as a tensor of u32 of dimension `N, C` where `N` is the batch size and `C` the number
/// * `target`: The ground truth labels as a tensor of u32 of dimension `N, C` where `N` is the batch size and `C` the number
/// of categories.
///
/// The resulting tensor is a scalar containing the average value over the batch.
Expand Down
4 changes: 2 additions & 2 deletions candle-nn/src/var_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ impl<'a> VarBuilder<'a> {
///
/// # Safety
///
/// The unsafe is inherited from [`memmap2::MmapOptions`].
/// The unsafe is inherited from [`candle::safetensors::MmapedSafetensors::multi`].
Copy link
Contributor Author

Choose a reason for hiding this comment

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

   --> /home/ivor/Documents/Code/rust/playground_candle/candle/candle-nn/src/var_builder.rs:500:40
    |
500 |     /// The unsafe is inherited from [`memmap2::MmapOptions`].
    |                                        ^^^^^^^^^^^^^^^^^^^^ no item named `memmap2` in scope
    |
    = note: `#[warn(rustdoc::broken_intra_doc_links)]` on by default

Presumably a copy paste from the documentation of multi itself, changed to point at the reason why this function is unsafe.

pub unsafe fn from_mmaped_safetensors<P: AsRef<std::path::Path>>(
paths: &[P],
dtype: DType,
Expand Down Expand Up @@ -589,7 +589,7 @@ impl ShardedSafeTensors {
///
/// # Safety
///
/// The unsafe is inherited from [`memmap2::MmapOptions`].
/// The unsafe is inherited from [`candle::safetensors::MmapedSafetensors::multi`].
pub unsafe fn var_builder<P: AsRef<std::path::Path>>(
paths: &[P],
dtype: DType,
Expand Down