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

implement a julia specific debug mode with --debug #53404

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KristofferC
Copy link
Member

This PR adds a new flag to control when a "julia specific" debug mode is active. When the debug mode is active the following happens:

  • The @assert macro is active
  • The isdebug function returns true.

Currently, the --debug has three options:

  • "yes": debug mode is active in scripts and packages
  • "script": debug mode is active in scripts (default)
  • "no": debug is not active anywhere

The debug mode is a "compile time option" meaning that it will recompile packages if it changes.

There are still many open questions regarding this:

  • Should this instead use -g and coexist with the current behavior of -g or should it just steal it?
  • Are the defaults here reasonable?
  • When running Pkg.test should assertions be enabled everywhere or only in the tested package etc?
  • Will it be expected that people run their tests under both debug and normal mode?
  • Should --debug has any influence on --check-bounds?
  • Probably many more

Subsumes #37874

Ref #41342, #53223

This PR adds a new flag to control when a "julia specific" debug mode is
active. When the debug mode is active the following happens:
- The `@assert` macro is active
- The `isdebug` function returns `true`.

Currently, the `--debug` has three options:
- `"yes"`: debug mode is active in scripts and packages
- `"script": debug mode is active in scripts
- `"no": debug is not active anywhere

The debug mode is a "compile time option" meaning that it will recompile
packages if it changes.
@KristofferC KristofferC added the feature Indicates new feature / enhancement requests label Feb 20, 2024
@vchuravy
Copy link
Member

Should this instead use -g and coexist with the current behavior of -g or should it just steal it?

-g is universally a flag for the level of debug information the compiler emits, it normally doesn't change assertions. In C -g doesn't change the behavior of assert.

For me these should be separate things (I may need to troubleshoot debug-information emission with ot without --debug)

@@ -218,6 +219,9 @@ julia> @assert isodd(3) "What even are numbers?"
```
"""
macro assert(ex, msgs...)
enable_asserts = !isdefined(Main, :Base) || Main.Base.julia_debug
Copy link
Member

Choose a reason for hiding this comment

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

!isdefined(Main, :Base)

I assume this is the canonical way to check if something is running as a script? possibly should factor that out into an independent iscurrentscript function somewhere

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not checking if it is a script actually, it is making sure that asserts are emitted when e.g. building the compiler (and Base is not defined).

Copy link
Member

Choose a reason for hiding this comment

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

gotcha

@adienes
Copy link
Member

adienes commented Feb 20, 2024

just to lightning round

Should this instead use -g

  • 👎

Are the defaults here reasonable?

  • 👍

Will it be expected that people run their tests under both debug and normal mode

  • 👎

Should --debug has any influence on --check-bounds

  • 🤔..👎

When running Pkg.test should assertions be enabled

only if Julia was started with --debug. Pkg.test should not override imo

Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

Looks sensible.

The only design alternative I see is to use a preference to allow fine-grained per package setting of this flag. E.g. a user may want to set debug for Enzyme, but not for anything else.

@@ -121,6 +121,23 @@ function setpropertyonce!(x::Module, f::Symbol, desired, success_order::Symbol=:
return Core.setglobalonce!(x, f, val, success_order, fail_order)
end

julia_debug::Bool = true
Copy link
Member

Choose a reason for hiding this comment

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

Makes me a bit nervous to have a compiler time setting that I can easily change by writing a global.

@KristofferC
Copy link
Member Author

The only design alternative I see is to use a preference to allow fine-grained per package setting of this flag. E.g. a user may want to set debug for Enzyme, but not for anything else.

In theory one could have something like --debug={Enzyme, ...} but I dunno...

@vchuravy
Copy link
Member

Yeah... I was thinking LocalPreferences.toml

[Enzyme]
debug = true

But we would need to reserve the preference name. Lookup without depending on Preferences.jl would be:

mod_uuid = get_uuid(__module__) # https://github.com/JuliaPackaging/Preferences.jl/blob/ae3184abaf7393097a4e31fede64f19ca5d3b1bc/src/utils.jl#L36
preferences = Base.get_preference(mod_uuid)
Base.record_compiletime_preference(mod_uuid, "debug")
local_debug = something(tryparse(Bool, get(preferences, "debug", "false")), false)

But that might be too extra for this.

@vchuravy
Copy link
Member

Probably many more

Your mentioning of Core.Compiler reminded me of this.

What should be the semantics w.r.t Base/Core? We could use JULIA_BUILD_MODE to a) default this new option to on; b) turn on assertions within Core and Base.

For the runtime we use FORCE_ASSERTIONS=1.

I have been wondering if we should provide pre-built binaries for JULIA_BUILD_MODE=debug + FORCE_ASSERTIONS=1 and make them available through juliaup

@JeffBezanson
Copy link
Member

👍
If this is really mostly about asserts maybe the name should be narrowed, e.g. --asserts? I do think it's better to have orthogonal flags (for debug info, check bounds, asserts, etc.). Or maybe something like --optional-checks?

@jariji
Copy link
Contributor

jariji commented Feb 25, 2024

I'm wondering if it would make sense to make it changeable at runtime. For example if I set up a function with a bunch of @asserts in it, call it a few times with test values for sanity checks with asserts enabled, and then disable assertions before passing it into a numerical optimizer where performance matters.

function f(x)
  @assert properties(x)
  sum(square, x)
end
@test f([0]) == 0
disabling_asserts() do 
  Optim.optimize(f, ...)
end

@KristofferC
Copy link
Member Author

KristofferC commented Feb 26, 2024

If this is really mostly about asserts maybe the name should be narrowed

Maybe, but this also adds an isdebug to allow @static if isdebug() (similar to #ifndef NDEBUG) so it isn't inherently only about asserts. Also, I wanted to try to limit the explosion of compile time options we have. But maybe for now this can just be an assert mode and then we also have --debug that enables --asserts and possible other debug specific options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Indicates new feature / enhancement requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants