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

Single CGC computation + locking on read #20

Merged
merged 3 commits into from
Jul 31, 2024

Conversation

LHerviou
Copy link
Contributor

As discussed with @lkdvos

Instead of computing the CGCs of all s3 when encountering s1 x s2 => s3, we just compute the CGCs of the relevant s3.
Following discussion, I added a lock on reading the CGC file. I am relatively convinced it is not useful.

Finally, I slightly modified the dist_cache_info function so it does not crash if some pid file are still there, or if some files are corrupted. Instead it simply reports them. I added an option to clean these files. I did not test it enough (due to ownership shenanigans).

* Updating the locking

* Update caching.jl

* Update caching.jl

Removed some debug options and cleaned some code

* Update caching.jl
Copy link

codecov bot commented Jul 31, 2024

Codecov Report

Attention: Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.

Files Patch % Lines
src/caching.jl 88.00% 3 Missing ⚠️
Files Coverage Δ
src/clebschgordan.jl 98.15% <100.00%> (-0.01%) ⬇️
src/caching.jl 89.10% <88.00%> (-0.18%) ⬇️

@lkdvos
Copy link
Member

lkdvos commented Jul 31, 2024

This looks great!
I just added some docstrings to reflect the changes, when the tests turn green I'll merge this and tag a new release.

I am also pretty sure the pidlock on read is not doing a whole lot, but in principle the scratchspace is shared per user, so if you would start up two sessions of Julia at the same time, it might still mess up? Honestly, I'd rather err on the cautious side with these things.

@lkdvos lkdvos merged commit 533f3c8 into QuantumKitHub:master Jul 31, 2024
5 of 8 checks passed
@LHerviou
Copy link
Contributor Author

Thanks for the help.
Some test turned red on one of the ubuntu (eof error in a jld2 file)

@lkdvos
Copy link
Member

lkdvos commented Jul 31, 2024

I also saw this, I am not sure if this is just the interplay with CI or an actual problem. Should I hold off on tagging, such that you can try it out for a while longer before I release this version?

@LHerviou
Copy link
Contributor Author

LHerviou commented Jul 31, 2024 via email

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