Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error in the specification of the sparse_factorize function. #113

Closed
jbaylina opened this issue Oct 4, 2021 · 12 comments
Closed

Error in the specification of the sparse_factorize function. #113

jbaylina opened this issue Oct 4, 2021 · 12 comments
Assignees

Comments

@jbaylina
Copy link

jbaylina commented Oct 4, 2021

The documentation says that w is the first column of m_hat or second column of m without the first row..

Actually it is the first column of m without the first row.

The implementation is ok: https://github.com/filecoin-project/neptune/blob/bafd77a5014e3b6a6b40359097835c3eb1dd533f/src/mds.rs#L197

@porcuquine
Copy link
Collaborator

I think this refers to the spec (https://github.com/filecoin-project/specs). @jbaylina Can you file there and tag @DrPeterVanNostrand who can fix? We can close this once it's accounted for there — unless I've misunderstood the issue.

@jbaylina
Copy link
Author

jbaylina commented Oct 7, 2021

I'll post there. But in this repo there is a pdf with spec. https://github.com/filecoin-project/neptune/blob/master/poseidon_spec.pdf

jbaylina added a commit to jbaylina/specs that referenced this issue Oct 7, 2021
Original reference: lurk-lab/neptune#113

The documentation says that w is the first column of m_hat or second column of m without the first row.. 

Actually it is the first column of m   without the first row.

The implementation is ok: https://github.com/filecoin-project/neptune/blob/bafd77a5014e3b6a6b40359097835c3eb1dd533f/src/mds.rs#L197
@porcuquine
Copy link
Collaborator

Ah, right. I believe that is generated from the spec. So once the spec is updated, we should regenerate the PDF and update here as well. Thanks.

@DrPeterVanNostrand
Copy link
Contributor

Thanks @jbaylina. I updated the spec pdf here: #114

@DrPeterVanNostrand
Copy link
Contributor

The change has also been added to the Filecoin spec here: filecoin-project/specs#1278

@DrPeterVanNostrand
Copy link
Contributor

@jbaylina I didn't see your specs PR before submitting my own

@DrPeterVanNostrand
Copy link
Contributor

The PDF in this repo is actually rendered using an app called Typora

@porcuquine
Copy link
Collaborator

The PDF in this repo is actually rendered using an app called Typora

I see. If the source isn't directly extracted from the spec, we should probably commit it along with (at least some manual) build instructions.

@DrPeterVanNostrand
Copy link
Contributor

DrPeterVanNostrand commented Oct 7, 2021

@porcuquine I've had the markdown in a local git repo since it was published. I just made a public git repo here: https://github.com/DrPeterVanNostrand/poseidon-spec

Basically, I make the necessary changes to my local markdown version, then render as a PDF via Typora, then PR the PDF to neptune. Then I PR the changes in MathJax to the Filecoin specs repo.

@porcuquine
Copy link
Collaborator

Got it. Do you mind just writing those instructions into the source repo (or make a directory for it here). That way if someone else needs to do it at some point, we at least know both how to produce the artifact and to make sure everything is in sync.

@DrPeterVanNostrand
Copy link
Contributor

Ok, I added the Markdown file and rendering instructions to PR #114

@porcuquine
Copy link
Collaborator

Merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants