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.

Sign up for GitHub

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

[ITensors] Fix ITensorsPackageCompilerExt precompile issue #1549

Closed
wants to merge 2 commits into from
Closed

[ITensors] Fix ITensorsPackageCompilerExt precompile issue #1549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
657ac91
Add VectorInterface extensions to PackageCompilerExt
kmp5VT Oct 18, 2024
8931772
Remove using statement
kmp5VT Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "ITensors"
uuid = "9136182c-28ba-11e9-034c-db9fb085ebd5"
authors = ["Matthew Fishman <[email protected]>", "Miles Stoudenmire <[email protected]>"]
version = "0.6.21"
version = "0.6.22"

[deps]
Adapt = "79e6a3ab-5dfb-504d-930d-738a2a938a0e"
Expand Down Expand Up @@ -39,7 +39,7 @@ ZygoteRules = "700de1a5-db45-46bc-99cf-38207098b444"
[extensions]
ITensorsHDF5Ext = "HDF5"
ITensorsObserversExt = "Observers"
ITensorsPackageCompilerExt = "PackageCompiler"
ITensorsPackageCompilerExt = ["PackageCompiler", "VectorInterface"]
ITensorsVectorInterfaceExt = "VectorInterface"
ITensorsZygoteRulesExt = "ZygoteRules"

Expand Down
1 change: 1 addition & 0 deletions ext/ITensorsPackageCompilerExt/precompile_itensors.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using ITensors.ITensorMPS: MPO, OpSum, dmrg, random_mps, siteinds
include("$(@__DIR__)/../ITensorsVectorInterfaceExt/ITensorsVectorInterfaceExt.jl")
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look like a good idea to do. Does it work if you load VectorInterface.jl to trigger the extension to load, i.e. add the line:

using VectorInterface: VectorInterface

It seems weird that we would have to do that though.

Copy link
Member

@mtfishman mtfishman Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I see in #1548 (comment) you said you already tried that and it didn't work.

The confusing part to me, and maybe what is causing problems here for some reason, is that VectorInterface.jl is a dependency of KrylovKit.jl, and KrylovKit.jl is a dependency of ITensors.jl, so ITensorsVectorInterfaceExt should get loaded automatically. So there is something strange happening with the order that the packages/extensions get loaded, I suspect it is a bug in Julia and/or Pkg.

Copy link
Member

Choose a reason for hiding this comment

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

Something you could try is adding VectorInterface.jl as a dependency of ITensors.jl directly and see if that forces ITensorsVectorInterfaceExt to get loaded. It's implicitly a dependency anyway, as I described, so it shouldn't be a problem adding it as a dependency, though it seems like a bug that we would have to do that.

One reason I made ITensorsVectorInterfaceExt is because when we move ITensors.ITensorMPS to ITensorMPS.jl, KrylovKit.jl and VectorInterface.jl will no longer be dependencies of ITensors.jl, so that extension was anticipating that code reorganization. But if it is causing problems that we can't circumvent right now we could move ITensorsVectorInterfaceExt into ITensors.jl and turn VectorInterface.jl into a dependency.


# TODO: This uses all of the tests to make
# precompile statements, but takes a long time
Expand Down
Loading