EnzymeAD / EnzymeAD/Enzyme.jl

Capitalization in EnzymeCore

Open
#2,164 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Julia
Stars
586
Forks
108
Avg merge
1d 5h
Merged PRs (30d)
44

Description

> [!WARNING]
> The following propositions are very breaking and not essential, so weigh them carefully.

Since a breaking release may be on the horizon, here are some of my gripes about capitalization and style. My reference is the official (albeit concise) [Julia style guide](https://docs.julialang.org/en/v1/manual/style-guide).

## Mode setters

> [Use naming conventions consistent with Julia `base/`](https://docs.julialang.org/en/v1/manual/style-guide/#Use-naming-conventions-consistent-with-Julia-base/):
> - modules and type names use capitalization and camel case: `module SparseArrays`, `struct UnitRange`.
> - functions are lowercase (`maximum`, `convert`) and, when readable, with multiple words squashed together (`isequal`, `haskey`). When necessary, use underscores as word separators.

Mode setters are functions. At the moment, some are already lowercase

```julia
clear_err_if_func_written
clear_runtime_activity
set_abi
set_err_if_func_written
set_runtime_activity
```

but some are uppercase and could be renamed (possibly while keeping the deprecated version around with a warning):

```julia
Combined => combined_mode
NoPrimal => noprimal_mode
Split => split_mode # split already exists
WithPrimal => with_primal
```

## Mode objects

> [Avoid confusion about whether something is an instance or a type](https://docs.julialang.org/en/v1/manual/style-guide/#Avoid-confusion-about-whether-something-is-an-instance-or-a-type)

To me, the following choices in Enzyme seemed rather confusing at first:

```julia
julia> ForwardMode isa Type
true

julia> Forward isa Type
false

julia> Forward isa ForwardMode
true
```

If renaming were easy, I would suggest either of the following conventions instead:

1. make mode instances all-uppercase like other module-level constants (recommendation found in [BlueStyle](https://github.com/JuliaDiff/BlueStyle?tab=readme-ov-file#global-variables) and [SciMLStyle](https://github.com/SciML/SciMLStyle?tab=readme-ov-file#globals-should-be-avoided-whenever-possible)

```julia
Forward => FORWARD
ForwardWithPrimal => FORWARD_WITH_PRIMAL
Reverse => REVERSE
ReverseWithPrimal => REVERSE_WITH_PRIMAL
ReverseHolomorphic => REVERSE_HOLOMORPHIC
ReverseHolomorphicWithPrimal => REVERSE_HOLOMORPHIC_WITH_PRIMAL
ReverseSplitNoPrimal => REVERSE_SPLIT_NO_PRIMAL
ReverseSplitWithPrimal => REVERSE_SPLIT_WITH_PRIMAL
```

2. obtain mode instances as the output of constructors, i.e. add parentheses

```julia
Forward => Forward()
ForwardWithPrimal => ForwardWithPrimal()
Reverse => Reverse()
ReverseWithPrimal => ReverseWithPrimal()
ReverseHolomorphic => ReverseHolomorphic()
ReverseHolomorphicWithPrimal => ReverseHolomorphicWithPrimal()
ReverseSplitNoPrimal => ReverseSplitNoPrimal()
ReverseSplitWithPrimal => ReverseSplitWithPrimal()
```

However I realize that this would break everyone's code everywhere, so this is obviously low priority unless there is community consensus.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.