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

rewrite constructbraidingtensor preprocessor #95

Merged
merged 5 commits into from
Jan 5, 2024

Conversation

Jutho
Copy link
Owner

@Jutho Jutho commented Dec 1, 2023

This is an alternative to PR#94, that should also be correct if you have a tensor expression composed of a sum of different tensor network contractions. I've included the test cases from PR#94.

I think also _remove_braidingtensors will suffer from the same bugs as the old _construct_braidingtensors, so I probably will also rewrite this one before merging.

Also, it needs some more testing and it contains a branch that should not active and currently prints out a "huh". So I should not forget to remove this.

Maybe you can already test with your code and with MPSKit: @lkdvos and @Gertian .

@Jutho
Copy link
Owner Author

Jutho commented Dec 1, 2023

It is still easy to break @planar by doing things like adding /2 or other scalar expressions in tensor expressions, so I will need to debug/rewrite further.

Copy link

codecov bot commented Dec 1, 2023

Codecov Report

Attention: 57 lines in your changes are missing coverage. Please review.

Comparison is base (bb2b4cf) 80.57% compared to head (dc552e9) 81.43%.

Files Patch % Lines
src/planar/preprocessors.jl 77.36% 55 Missing ⚠️
src/planar/analyzers.jl 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   80.57%   81.43%   +0.85%     
==========================================
  Files          42       42              
  Lines        5453     5533      +80     
==========================================
+ Hits         4394     4506     +112     
+ Misses       1059     1027      -32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Gertian
Copy link
Contributor

Gertian commented Dec 4, 2023

@Jutho : Soon I'll have some fermionic PEPS that I want to contract and not optimize. This should be a great testcase :)

For now, both yours and Lukas fixes run but I don't know if the output is the correct one.

src/planar/planaroperations.jl Outdated Show resolved Hide resolved
src/planar/planaroperations.jl Outdated Show resolved Hide resolved
src/planar/preprocessors.jl Outdated Show resolved Hide resolved
@Jutho Jutho force-pushed the constructbraiding_rewrite branch from 00d1d23 to dc552e9 Compare January 5, 2024 07:56
@Jutho
Copy link
Owner Author

Jutho commented Jan 5, 2024

When the test turn green, I think this is now ready. I have locally tested the current master of MPSKit with this on Julia 1.10, and all seems fine. Maybe there are additional tests from PEPSKit that @Gertian wants to try?

@Jutho
Copy link
Owner Author

Jutho commented Jan 5, 2024

Ok I will merge this. If there would still be issues with this in PEPSKit.jl @Gertian , let me know.

@Jutho Jutho merged commit b62a772 into master Jan 5, 2024
12 of 13 checks passed
@Jutho Jutho deleted the constructbraiding_rewrite branch January 5, 2024 10:24
@Gertian
Copy link
Contributor

Gertian commented Jan 7, 2024

@Jutho I'll see if my old code for contraction still works.

What is the current status for AD of planar fermionic diagrams ?

@lkdvos
Copy link
Collaborator

lkdvos commented Jan 7, 2024

Somewhere near the top of my ever larger to-do list 😉 I'm a little hesitant to promise due dates, but maybe end of January-ish? Maybe @Jutho has more time?

@Gertian
Copy link
Contributor

Gertian commented Jan 7, 2024

Great :)

I'm happy to hear it's still on your menu !

@Jutho
Copy link
Owner Author

Jutho commented Jan 8, 2024

Definitely still on the menu. I have a lot of grading to do the coming weeks, but if I need some distraction or procrastination, the AD rules for the planar operations might be a good candidate. However, you are also very welcome to give it a try yourself if you want :-).

@Gertian
Copy link
Contributor

Gertian commented Jan 17, 2024

To be honest I have no idea where to get started with this.

@lkdvos
Copy link
Collaborator

lkdvos commented Jan 30, 2024

https://github.com/Jutho/TensorKit.jl/tree/ld/planar-ad

This was a little more painful than I anticipated, but I think this could be a preliminary working version. Can you try it out? I wrote some small tests, but I can't say I am super confident about it.

@Gertian
Copy link
Contributor

Gertian commented Feb 4, 2024

Thanks, I'll play around with it today.
I'll let you know what I found (or didn't find)

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.

3 participants