Skip to content

Check md5 integrity against vp-setupfit in n3fit#2462

Merged
achiefa merged 10 commits into
masterfrom
md5_issue
May 8, 2026
Merged

Check md5 integrity against vp-setupfit in n3fit#2462
achiefa merged 10 commits into
masterfrom
md5_issue

Conversation

@achiefa
Copy link
Copy Markdown
Contributor

@achiefa achiefa commented Apr 28, 2026

This PR should address #2461. Please have a look.

Also, I was wondering whether we want to hash also the files in filter directory. This would be an easy fix.

@achiefa achiefa requested review from jacoterh and scarlehoff April 28, 2026 13:29
@achiefa achiefa changed the base branch from master to diag_covmat_reproducibility April 28, 2026 13:30
@achiefa
Copy link
Copy Markdown
Contributor Author

achiefa commented Apr 29, 2026

I think I'm done with this. Can you please check so that we can merge it into Jaco's PR?

@jacoterh jacoterh force-pushed the diag_covmat_reproducibility branch from 3bb2b7f to 56d001c Compare April 30, 2026 14:52
@achiefa achiefa changed the title Update md5 to account for stored covmat Check md5 integrity against vp-setupfit in n3fit May 5, 2026
@achiefa achiefa force-pushed the md5_issue branch 4 times, most recently from 962e8fe to 4581109 Compare May 5, 2026 12:48
@achiefa
Copy link
Copy Markdown
Contributor Author

achiefa commented May 5, 2026

I tested this and it works fine. Should I remove the hashing of the theorycovamat?

@scarlehoff
Copy link
Copy Markdown
Member

Should I remove the hashing of the theorycovamat?

Yes, since we are not going to recompute the theorycovmat when running n3fit.

Comment thread n3fit/src/n3fit/scripts/n3fit_exec.py Outdated
Comment thread n3fit/src/n3fit/scripts/n3fit_exec.py Outdated
@achiefa
Copy link
Copy Markdown
Contributor Author

achiefa commented May 7, 2026

Hi @scarlehoff, I've implemented your suggestions. Let me know.

@scarlehoff
Copy link
Copy Markdown
Member

I think you can rebase only the md5 part on top of master (instead of pointing to diag_covmat_reproducibility) and we merge this by itself.
It's mostly orthogonal and it can be useful already I would say.

@achiefa achiefa changed the base branch from diag_covmat_reproducibility to master May 7, 2026 11:44
@achiefa
Copy link
Copy Markdown
Contributor Author

achiefa commented May 7, 2026

Yes, I agree. I've reversed the changes inherited from diag_covmat_reproducibility. Now this points to master. Once the tests finish, we can merge this.

Edit: let me rebase on top of master before

@achiefa
Copy link
Copy Markdown
Contributor Author

achiefa commented May 7, 2026

Rebased on top of master

@achiefa
Copy link
Copy Markdown
Contributor Author

achiefa commented May 7, 2026

I've updated the tests, and the documentation as well. The tests are now running. Can you please check if this is what you had in mind?

Copy link
Copy Markdown
Member

@scarlehoff scarlehoff left a comment

Choose a reason for hiding this comment

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

Thanks! And thanks for updating the docs as well.

@achiefa achiefa merged commit 0f0dd49 into master May 8, 2026
12 checks passed
@achiefa achiefa deleted the md5_issue branch May 8, 2026 12:52
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

Successfully merging this pull request may close these issues.

2 participants