WIP: scheduler integration #1460
Conversation
|
i updated this with the integration rust-fil-proofs -> bellperson. seems there are some errors when starting the scheduler in CI. . i just updated the scheduler to show a more verbose error message. |
| pub_params: &PublicParams<'a, S>, | ||
| pub_in: &S::PublicInputs, | ||
| priv_in: &S::PrivateInputs, | ||
| groth_params: &'b groth16::MappedParameters<Bls12>, | ||
| post_type: Option<BellTaskType>, |
There was a problem hiding this comment.
CompoundProof is an abstraction that doesn't (and shouldn't) know anything whatsoever about specific proof types. So mention of post_type here, MerkleTree later, etc. is by definition inappropriate. It might be that post_type is just misnamed (will look more closely) — but the point stands: if we are referencing specific proof types of any kind here then something is wrong.
There was a problem hiding this comment.
(unrelated to @porcuquine's comment) It looks like you always pass in a type, so I don't think it needs to be wrapped in a Option.
There was a problem hiding this comment.
CompoundProofis an abstraction that doesn't (and shouldn't) know anything whatsoever about specific proof types. So mention ofpost_typehere,MerkleTreelater, etc. is by definition inappropriate. It might be thatpost_typeis just misnamed (will look more closely) — but the point stands: if we are referencing specific proof types of any kind here then something is wrong.
@porcuquine I did not like the idea of refactoring that trait neither, as it seems to be a super Trait from which other traits inherit. but, at the moment it looked like the cleaning way to modify and pass the information as a task_type(which at the moment encodes deadlines and possible exclusive resources) to the scheduler, without having to refactor many other places.
@vmx Initially, it was defined as an optional param, I set it to MerkeProof, but I am not sure if that type should encompass all the processes that use the GPU but that are not Winning/Window post. It is important because all those processes will end up using the GPU assigned to MerkeTree operations.
| preemptible: self.num_iter > 1, | ||
| }; | ||
| let task_req = TaskReqBuilder::new() | ||
| .with_task_type(TaskType::MerkleProof) |
There was a problem hiding this comment.
Assuming this type is meant to describe Merkle tree building, it is misnamed. Merkle proofs are later generated over these trees, but the act of building them shouldn't reference 'proof', since that is confusing.
There was a problem hiding this comment.
I am open to set a more appropriate name here. also not sure if it should include the word Merkle as what I have seen is that there are operations that not only use neptune but bellperson.
use None as a type for seal - use latest scheduler and bellperson - Make the scheduler an optional dependency available under the gpu feature - include the proof_ext module only if the gpu flag is enable - remove scheduler dependency using bellperson task_type - import the scheduler if gpu flag is set comment out logger to fix clippy warning Remove BatcherType aligning to latest changes in neptune switch back to neptune base repository reduce timeout to 20 min feat: Use the context tool to improve logging messages feat: use the optional task name to improve logging fix: remove sync iterator as it was a typo during testing
integration with the scheduler
|
This effort has been stopped, hence closing this PR. |
This PR is a work-in-progress and must not be used in production
Integrates the scheduler into the current code trying to keep untouched as much code as possible:
some test fails, meaning something is wrong, so this PR is just for getting an insight about the problem source, do not use in production yet.