Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Conversation

10to4
Copy link
Contributor

@10to4 10to4 commented Jul 25, 2024

Add ccs backend
Add Fibonacci example for ccs and hypernova from sonobe

@10to4 10to4 requested a review from leolara July 25, 2024 10:13
@10to4
Copy link
Contributor Author

10to4 commented Aug 1, 2024

I noted the details of the key parts of the CCS backend code.
https://hackmd.io/oii0lSx6RSGX2lCa0AYimw

@leolara
Copy link
Collaborator

leolara commented Aug 9, 2024

There are two representations, ir::Circuit and backend::ccs::CCSCircuit. Is this second one only specific for Sonobe?

  1. The new and then write is not the right pattern for intermediate representations. There should be a new that receives all the info to create it. You are calling new and then right away write with more info. It should be in the same call to new just do this call later.
  2. The IR it is just representation without logic. So the methods calcu_bounds and construct_matrix_coeffs_and_offsets should be part of the compiler, and you have the result already when you are calling ir::Circtuit::new
  3. Is backend/ccs.rs part of sonobe or part of the IR represnation. we should have in IR things that are common for all CCS, and in backend only things that are part of sonobe. Each submodule of backend should be a backend, so if you need to separete a module, it should be inside backend::sonobe pacakge, not inside backend.

@leolara
Copy link
Collaborator

leolara commented Aug 9, 2024

Why is not passing the tests?

@10to4
Copy link
Contributor Author

10to4 commented Aug 9, 2024

There are two representations, ir::Circuit and backend::ccs::CCSCircuit. Is this second one only specific for Sonobe?

  1. The new and then write is not the right pattern for intermediate representations. There should be a new that receives all the info to create it. You are calling new and then right away write with more info. It should be in the same call to new just do this call later.
  2. The IR it is just representation without logic. So the methods calcu_bounds and construct_matrix_coeffs_and_offsets should be part of the compiler, and you have the result already when you are calling ir::Circtuit::new
  3. Is backend/ccs.rs part of sonobe or part of the IR represnation. we should have in IR things that are common for all CCS, and in backend only things that are part of sonobe. Each submodule of backend should be a backend, so if you need to separete a module, it should be inside backend::sonobe pacakge, not inside backend.

@leolara Okay, got it.
backend::ccs::CCSCircuit is based on the CCS structure defined in the paper, not Sonobe. Because no matter which backend library, the structure definition of CCS is very similar, and the difference is minimal, so a CCSCircuit is defined in the backend.
Originally, considering that when connecting to different backend libraries, there will be minimal repetition in transforming directly from CCSCircuit in the backend module to backend libraries, no matter Sonobe or other repos in the future.

@10to4
Copy link
Contributor Author

10to4 commented Aug 9, 2024

Why is not passing the tests?

@leolara There are errors while compiling serde lib. To be compatible with Sonobe, I modify the rust-toolchain from nightly-2024-02-14 to nightly-2024-02-13.

@leolara
Copy link
Collaborator

leolara commented Aug 13, 2024

There are two representations, ir::Circuit and backend::ccs::CCSCircuit. Is this second one only specific for Sonobe?

  1. The new and then write is not the right pattern for intermediate representations. There should be a new that receives all the info to create it. You are calling new and then right away write with more info. It should be in the same call to new just do this call later.
  2. The IR it is just representation without logic. So the methods calcu_bounds and construct_matrix_coeffs_and_offsets should be part of the compiler, and you have the result already when you are calling ir::Circtuit::new
  3. Is backend/ccs.rs part of sonobe or part of the IR represnation. we should have in IR things that are common for all CCS, and in backend only things that are part of sonobe. Each submodule of backend should be a backend, so if you need to separete a module, it should be inside backend::sonobe pacakge, not inside backend.

@leolara Okay, got it. backend::ccs::CCSCircuit is based on the CCS structure defined in the paper, not Sonobe. Because no matter which backend library, the structure definition of CCS is very similar, and the difference is minimal, so a CCSCircuit is defined in the backend. Originally, considering that when connecting to different backend libraries, there will be minimal repetition in transforming directly from CCSCircuit in the backend module to backend libraries, no matter Sonobe or other repos in the future.

Ok, so my interpretation of what you say is that CCSCircuit is the actual IR of CCS, what is the different with the current IR. Why do we need to do it in two steps?

Ideally they would be merged

@10to4
Copy link
Contributor Author

10to4 commented Aug 13, 2024

There are two representations, ir::Circuit and backend::ccs::CCSCircuit. Is this second one only specific for Sonobe?有两种表示形式,ir::Circuit 和 backend::ccs::CCSCircuit。这第二个只针对 Sonobe 吗?

  1. The new and then write is not the right pattern for intermediate representations. There should be a new that receives all the info to create it. You are calling new and then right away write with more info. It should be in the same call to new just do this call later.对于中间表示来说,新的 and then write 不是正确的模式。应该有一个新的接收所有信息以创建它。您正在呼叫新的,然后立即写下更多信息。它应该在对 new 的同一调用中,稍后再进行此调用。
  2. The IR it is just representation without logic. So the methods calcu_bounds and construct_matrix_coeffs_and_offsets should be part of the compiler, and you have the result already when you are calling ir::Circtuit::new这 IR 它只是没有逻辑的表示。因此,calcu_bounds 和 construct_matrix_coeffs_and_offsets 方法应该是编译器的一部分,并且在调用 ir::Circtuit::new 时,您已经有了结果
  3. Is backend/ccs.rs part of sonobe or part of the IR represnation. we should have in IR things that are common for all CCS, and in backend only things that are part of sonobe. Each submodule of backend should be a backend, so if you need to separete a module, it should be inside backend::sonobe pacakge, not inside backend.backend/ccs.rs 是 sonobe 的一部分还是 IR 响应的一部分。我们应该在 IR 中拥有所有 CCS 通用的东西,而在后端只拥有属于 SONOBE 的一部分。后端的每个子模块都应该是一个后端,所以如果你需要分离一个模块,它应该在后端内部::sonobe pacakge,而不是后端内部。

@leolara Okay, got it. backend::ccs::CCSCircuit is based on the CCS structure defined in the paper, not Sonobe. Because no matter which backend library, the structure definition of CCS is very similar, and the difference is minimal, so a CCSCircuit is defined in the backend. Originally, considering that when connecting to different backend libraries, there will be minimal repetition in transforming directly from CCSCircuit in the backend module to backend libraries, no matter Sonobe or other repos in the future.好的,知道了。backend::ccs::CCSCircuit 基于论文中定义的 CCS 结构,而不是 Sonobe。因为无论哪个后端库,CCS的结构定义都非常相似,而且差异很小,所以在后端定义了一个CCSCircuit。本来,考虑到在连接到不同的后端库时,无论将来使用 Sonobe 还是其他仓库,在后端模块直接从 CCSCircuit 转换到后端库时,重复次数都会很小。

Ok, so my interpretation of what you say is that CCSCircuit is the actual IR of CCS, what is the different with the current IR. Why do we need to do it in two steps?好的,所以我对你说的解释是,CCSCircuit是CCS的实际IR,与当前的IR有什么不同。为什么我们需要分两步来做?

Ideally they would be merged理想情况下,它们将被合并

Yes, I got it. I am merging these parts now.

@10to4 10to4 force-pushed the 207-investigate-how-to-compile-chiquito-style-state-machine-to-ccs branch from 454d634 to 6858064 Compare August 27, 2024 10:46
}

// input vector z = [1, x, w]
pub struct Z<F: Field + From<u64>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@10to4 is Z pub or pub(crate) or pub(super), if it is pub it needs more documentation and explanation of what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

}
}

pub fn write(
Copy link
Collaborator

Choose a reason for hiding this comment

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

@10to4 there should be a way to create Z with an intitial value. If you are always calling new and then write they should be the same method

Copy link
Collaborator

Choose a reason for hiding this comment

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

If Z is a container, perhaps some of this logic should be outside, this seems that you are converting something into Z, so writeis not the right name, and perhaps you need a method that is not a member, and creates this from scratch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

}
}

pub fn write(&mut self, values: &[(usize, usize, F)]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again write is only necessary if Matrix is modified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

self.n - self.l - 1
}

pub fn write(
Copy link
Collaborator

Choose a reason for hiding this comment

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

@10to4 write is only necessary if this is modified, but it should not be modified as it is an IR. It is created by the compiler and then read by the backend

}

#[derive(Debug, Default)]
pub struct CCSCircuit<F> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is an IR should have not so much logic, it is just a container for informaation, the compiler should create this information in the unit and then set it in this IR.

@leolara
Copy link
Collaborator

leolara commented Aug 27, 2024

@10to4 I think you put a log of logic in the IR that belongs to the compiler. Look how plonkish IR code is just containing the IR, but not the logic to create the information in it: https://github.com/privacy-scaling-explorations/chiquito/blob/chiquito-2024/src/plonkish/ir/mod.rs

@10to4 10to4 force-pushed the 207-investigate-how-to-compile-chiquito-style-state-machine-to-ccs branch from 27a10c7 to da8a22d Compare August 27, 2024 14:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants