[post frontend-backend split] Discuss names for new types / functions

[ed255]

With the frontend-backend split, new types and functions have been introduced that mirror existing functions and types. Let's discuss the best naming for those.

This is the status of the initial frontend-backend PR:

Split VS Non-Split API

• Non-Split
  • halo2_proofs::plonk::create_proof
  • halo2_proofs::plonk::keygen_pk
  • halo2_proofs::plonk::keygen_vk
• Split
  • halo2_backend::plonk::prover::{ProverV2Single, ProverV2}
  • halo2_backend::plonk::keygen::keygen_pk_v2
  • halo2_backend::plonk::keygen::keygen_vk_v2

New middleware types (not mirroring)

• halo2_middleware::circuit::CompiledCircuitV2
• halo2_middleware::circuit::PreprocessingV2

Simplified types that mirror types that have frontend/backend specific behaviour

• Middleware
  • halo2_middleware::circuit::ConstraintSystemV2Backend
  • halo2_middleware::circuit::GateV2Backend
  • halo2_middleware::circuit::ExpressionMid
  • halo2_middleware::circuit::FixedQueryMid
  • halo2_middleware::circuit::AdviceQueryMid
  • halo2_middleware::circuit::InstanceQueryMid
  • halo2_middleware::circuit::ChallengeMid
  • halo2_middleware::circuit::ColumnMid
  • halo2_middleware::lookup::ArgumentV2
  • halo2_middleware::shuffle::ArgumentV2
  • halo2_middleware::permutation::ArgumentV2
  • halo2_middleware::permutation::AssemblyMid
• Frontend/Backend specific
  • halo2_common::plonk::Gate
  • halo2_common::plonk::ConstraintSystem
  • halo2_common::plonk::circuit::Expression
  • halo2_common::plonk::circuit::FixedQuery
  • halo2_common::plonk::circuit::AdviceQuery
  • halo2_common::plonk::circuit::InstanceQuery
  • halo2_common::plonk::circuit::Challenge
  • halo2_common::plonk::circuit::Column
  • halo2_common::plonk::lookup::Argument
  • halo2_common::plonk::shuffle::Argument
  • halo2_common::plonk::permutation::Argument
  • halo2_common::plonk::permutation::AssemblyFront (NOTE: This should be moved to the frontend)
  • halo2_backend::plonk::permutation::keygen::Assembly

Some initial discussion:

As you can see, the current naming is not fully consistent, so it can be improved!

Discussion 1: Suffixes or not? Since the mirrored types live under different paths, we could use colliding names. For example we could have

• halo2_middleware::circuit::Gate
• halo2_common::plonk::Gate
  This has the benefit of not having to agree on the suffix to use, but it has some drawbacks:
• Code that imports the two (this should only happen in halo2 node, not on circuit code), will either need to use full paths, or import with changed names use ..::Gate as GateMid
• When reviewing halo2 code, when we se Gate it's not clear which one it is
  Considering these drawbacks, I think it's better to use suffixes

Dicussion 2: For all the types that are mirrored -> simplified, use the same prefix when they appear in the middleware. For example all mirrored types under middleware (seen in the 3rd group list) would have Mid as suffix, or another suffix we agree upon. So ArgumentV2 would be ArgumentMid and so on. Can you think of a better suffix?

Discussion 3: Non-split VS split API. Should we distinguish the new API with some suffix? Usually code will either use the old API or the new API, not both, so I think it's viable to have this, without being confusing:

• Non-Split
  • halo2_proofs::plonk::create_proof
  • halo2_proofs::plonk::keygen_pk
  • halo2_proofs::plonk::keygen_vk
• Split
  • halo2_backend::plonk::prover::{ProverSingle, Prover}
  • halo2_backend::plonk::keygen::keygen_pk
  • halo2_backend::plonk::keygen::keygen_vk

---

Blocked by #254

[adria0]

Some thoughts:

• Since we are splitting HALO2 in order to support different frontends and backends, eventually we will have frontend_xxxx and backend_yyyy, although also temporally, for now, frontend and backend will be just a good naming. So middleware is what is expected not going to change, so, I'm on NOT suffix middleware, if we have to suffix something because this is going to be the invariant over time.

• Anyway in general, I find suffix/prefix a little verbose when you are implementing the structs/functions that they are used in the same module, so I'm on letting the developer if wants to use middleware::Gate as GateMid or just middleware::Gate.

• About the "should we distinguish the new API with some suffix?": I do not get what are you referring to... using different crates is a way to suffixing new API, is this about suffixing names or at module level?

[ed255]

Some thoughts:

Thanks for your thoughts!

• Since we are splitting HALO2 in order to support different frontends and backends, eventually we will have frontend_xxxx and backend_yyyy, although also temporally, for now, frontend and backend will be just a good naming. So middleware is what is expected not going to change, so, I'm on NOT suffix middleware, if we have to suffix something because this is going to be the invariant over time.

That makes sense!

• Anyway in general, I find suffix/prefix a little verbose when you are implementing the structs/functions that they are used in the same module, so I'm on letting the developer if wants to use middleware::Gate as GateMid or just middleware::Gate.

Regarding that, I'm afraid of confusing the developer when working with the codebase, because they may see a Gate and not know which one it is at first glance. They may see a function that requires a Gate and have a mental overhead to figure out which one it must be. I guess doing middleware::Gate as GateMid would be better, but we may end up with a mixed style, where we rename to avoid collision, but sometimes we only import middleware::Gate without renaming. With the suffix approach it's easy to grep for GateMid and get all places where it's used.

• About the "should we distinguish the new API with some suffix?": I do not get what are you referring to... using different crates is a way to suffixing new API, is this about suffixing names or at module level?

What I mean is that currently we have these:

• halo2_backend::plonk::prover::{ProverV2Single, ProverV2}
• halo2_backend::plonk::keygen::keygen_pk_v2
• halo2_backend::plonk::keygen::keygen_vk_v2

where the v2 is used to distinguish that this is the frontend-backend split API. But they could be:

• halo2_backend::plonk::prover::{ProverSingle, Prover}
• halo2_backend::plonk::keygen::keygen_pk
• halo2_backend::plonk::keygen::keygen_vk

And I think the latter is better. Even though keygen_pk, keygen_vk collides in name with the old API, they are never mixed in the same code.

[duguorong009]

@ed255
I believe this issue is no longer needed, since we already merged frontend-backend split PRs.
How about closing the issue as Done?

cc @davidnevadoc

[ed255]

@ed255 I believe this issue is no longer needed, since we already merged frontend-backend split PRs. How about closing the issue as Done?

cc @davidnevadoc

Actually there was a missing thing, I just opened this PR #312 to resolve it, and after that we can close this issue :)