stan-dev / stan-dev/math

Having a logger with levels?

Open
#2,545 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
839
Forks
220
Avg merge
2d 4h
Merged PRs (30d)
14

Description

Description

I think it would be nice to have a global logger or something of the sort that only does certain checks if a log level is set.

This comes from this discourse post where the user was effectively calling sqrt(0). This doesn't look bad to a Stan user, but for autodiff the adjoint calculation requires dividing by the input value and so we end up doing a divide by zero!

I think it would be nice to have a global logger that we don't pay for unless the user asks for it.

Example

For log levels I think we could have something like

0: No checks
1: (Default) perform a set of default checks (like ldlt checks) or checks that are very low cost
2: The checks from (1) as well as checks for correctness (such as sqrt for vars not taking zero valued input)
3: The checks from (2) as well as checks for nans and other nitty gritty checks we may want

As per code I'm thinking of something like the below, where logger is a class that holds a writer to an error stream. It's function check takes in a lambda that returns a value of true or false, the variable to check, the name of the function, and an error message.


// Somewhere in stan math namespace inititialize the logger
namespace stan {
namespace math {
static logger = init_logger<STAN_LOG_LEVEL>();
}
}

inline var sqrt(const var& a) {
  // Check if input is zero
  logger.check([](auto&& x) { return x.val() != 0.0}, a, "sqrt",
   "Input cannot be zero! Adjoint calculation divides by the input value.");
  // Check if input is nan
  // Maybe we call this one a lot so we make a special function
  logger.check_nan(a, "sqrt");

  return make_callback_var(std::sqrt(a.val()), [a](auto& vi) mutable {
    a.adj() += vi.adj() / (2.0 * vi.val());
  });
}

By setting the debug level at compile time we can turn those check functions into static no-ops the compiler should be able to remove so we don't pay for them.

Does anyone have thoughts on this? tbh I'm not sure if we should roll our own or utilize an existing library. I also am unsure about whether we should throw for these checks or allow the program to continue.

The other question is whether this would go in Stan math or stanc3. Thinking about this more now this actually seems like something we could add in stanc3. Then we would have a compiler flag like --log-level=# and instead of the program printing

var x = sqrt(y);

For each function on the rhs it would look up a list of checks that need to be done for certain log levels and just use the check functions available in math

check_nonzero(y, "sqrt", ...);
var x = sqrt(y);

Expected Output

More meaningful error messages for user errors

Current Version:

v4.1.0

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the proposed logger, the var sqrt path, and whether the work belongs in Stan Math or stanc3; the issue names no files or tests. A complete change would define log levels, decide compile-time versus runtime behavior and error handling, and produce more meaningful user-error messages without paying for disabled checks.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.