refactor!: clarify kw/positional parameters

[PierreQuinton]

No description provided.

[ValerianRey]

I agree with all of these changes. Need changelog entry explaining how to make the change.

@claude Could you please check that no parameter, similar to those that were changed, was forgotten?

[PierreQuinton]

The autojac ones are probably more debatable, I took the minimal decision here but we could make it stricter in some cases. I think that as long as a parameter is called tensor or output it can be positional only. Similarly, for booleans and chunk-sizes it makes sense to have them named-only.

EDIT: Not sure what happened in my backward commit, but there was a change you need to check there @ValerianRey

[PierreQuinton]

@claude what do you think of the changes on the public interface of the autojac package I just made (the last 4 commits).

[claude]

Claude finished @PierreQuinton's task in 53s —— View job

---

Review of autojac interface changes (last 4 commits)

• Gather context
• Read changed autojac files
• Form and post review

---

The 4 commits apply a "minimal" positional/named-only separation to backward, jac, jac_to_grad, and mtl_backward. Here's my assessment:

What works well

The split between the three groups is logically motivated:

• Positional-only (/): the primary tensor input(s) — good, these are the subject of the call.
• Named-only (*): boolean flags and chunk sizes — very good, calling backward(loss, retain_graph=True) reads much better than the positional equivalent.
• Middle group (positional-or-keyword): the remaining parameters.

The debatable middle group — per function

backward(tensors, /, jac_tensors, inputs, *, retain_graph, parallel_chunk_size)

jac_tensors and inputs sit in between. You already noted in the commit message that tensors could be the only positional and the rest named-only. I'd agree with that stricter split: jac_tensors and inputs are optional refinements of the call, not primary subjects, and there's little benefit in allowing them to be passed positionally (the caller has to count carefully: "is inputs the second or third arg?"). Making them named-only would be consistent with how torch.autograd.backward itself treats inputs (named-only since PyTorch 2.0).

jac(outputs, /, inputs, jac_outputs, *, retain_graph, parallel_chunk_size)

Same reasoning. inputs and jac_outputs read clearly as keyword arguments: jac(outputs, inputs=params, jac_outputs=init_jac) vs the positional equivalent. Making them named-only seems clearly better here.

jac_to_grad(tensors, /, aggregator, *, retain_jac)

aggregator is interesting. It's a required argument and the call site always looks like jac_to_grad(tensors, UPGrad()), which reads fine positionally. On the other hand, it's conceptually a configuration of how to reduce, not a "second input tensor". Either choice is defensible. Keeping it in the middle (as done here) is reasonable.

mtl_backward(tensors, /, features, grad_tensors, tasks_params, shared_params, *, retain_graph, parallel_chunk_size)

features is the most natural candidate for positional-only alongside tensors — it's the other primary tensor input to the function (you mentioned this in the comment). The rest (grad_tensors, tasks_params, shared_params) are optional overrides and would read more clearly as named-only. The current minimal split places all four in the middle, which is fine but leaves room for the stricter version you described.

Overall verdict

The minimal separation is a safe, non-breaking step in the right direction and the named-only treatment of booleans/chunk-sizes is unambiguously correct. The middle group decisions are all defensible, though the stricter version (moving jac_tensors/inputs and jac_outputs to named-only, and features to positional-only in mtl_backward) would be cleaner API design. Since the commit messages explicitly acknowledge this tradeoff, the current state is a conscious choice rather than an oversight.

[ValerianRey]

The autojac ones are probably more debatable, I took the minimal decision here but we could make it stricter in some cases. I think that as long as a parameter is called tensor or output it can be positional only. Similarly, for booleans and chunk-sizes it makes sense to have them named-only.

That's the version I prefer I think. More positional-only parameters (like inputs and jac_outputs) would be less readable IMO. More keyword-only parameters (like inputs and jac_outputs) would be annoying in some cases I guess. Like when you just want jac(outputs, inputs), which is extremely readable already.

IMO, we have several ways to use the autojac functions. The simplest one is with only outputs (or tensors) specified. So it makes sense that this is pos-only. But in some cases, when we need the inputs param or the jac_outputs param, these params are still central to the function, so I think that in a way, it makes sense to have them pos-and-kw.

Arguably, we could have inputs, jac_outputs, shared_params and tasks_params be keyword-only, as these are optional. I'm not sure. I don't like the reduced compatibility with torch though.

I would favor keeping things as you did, i.e. not very strict.

[PierreQuinton]

I would go for strict versions for all except jac_to_grad (I like aggregator=, it is a functionality that we give to the function). I think the debate is

• backward compatibility (but somehow I feel like we want to break it as much as we could ever desire in this release.
• torch compatibility: @ValerianRey mentioned that he would like to be able to convert autograd.* calls to autojac.* calls by simply changing the name of the calling function an nothing else. This is doable if the jac_* parameters are the default. I would argue that with the new release, users will anyway have to use the jac_to_grad function after a call to a autojac.*backward function so we are slightly drifting away from torch. I don't think this is prohibitive since a user that made the effort of installing the library will make the little effort of fixing that error (I'm pretty sure even Grok can catch this).

[ValerianRey]

About jac_to_grad: I think it's good to have the aggregator pos-and-kw. Because we either want to do:

aggregator = UPGrad()
...
jac_to_grad(params, aggregator)

or

jac_to_grad(params, aggregator=UPGrad())

[PierreQuinton]

I think we agree on everything and the line is thin, my preference is small. Let's take time until tomorrow to think this through and decide then.

[ValerianRey]

Yeah so let's have inputs, jac_outputs (or analog), shared_params and tasks_params as kw-only.
I'm not sure about features. I don't like it as pos-only, because it's not very understandable. But it's not an optional param, so it's a bit different from the others that we talked about here. I think kw-only is fine for it too if we make everything else also kw-only.

[ValerianRey]

Btw i'm not sure what kind of conventional commit this should be. As far as I understand, style is mostly for minor style changes in the code or docs. @claude what's supposed to be the conventional commit here?

[claude]

Claude finished @ValerianRey's task in 27s —— View job

---

• Gather context
• Answer the question

---

Conventional commit type for this PR

style: is not the right type here. In conventional commits, style means changes that do not affect the meaning of the code — whitespace, formatting, missing semicolons, etc. Adding / and * to Python function signatures is a semantic change: it restricts how callers can pass arguments and can break existing call sites that use keyword syntax for positional-only params (or vice versa).

The more appropriate types:

• refactor: — restructuring code without adding features or fixing bugs. This fits if the intent is to formalize the already-intended calling convention. If it's potentially breaking, append !: refactor!:
• fix: — if you consider the previous signatures as accidentally too permissive (a latent bug in the API contract), though this is a stretch.

Given that the discussion explicitly mentions "we want to break it as much as we could ever desire in this release," I'd go with:

refactor!: clarify positional/named-only parameters in public interface

The ! signals a breaking change, which is accurate since callers passing positional args as keyword args (or keyword-only args positionally) will get TypeErrors after this change.

[ValerianRey]

I think this is probably as good as we can get.