ACEsuit / ACEhamiltonians.jl

Other
13 stars 7 forks source link

Reproduce plots from the paper #30

Closed computative closed 1 year ago

computative commented 1 year ago

My goal is to reproduce fig 3 of [Zhang, L., Onat, B., Dusson, G. et al. Equivariant analytical mapping of first principles Hamiltonians to accurate and transferable materials models. npj Comput Mater 8, 158 (2022). https://doi.org/10.1038/s41524-022-00843-2]. I have started with the arXiv.2111.13736-branch of ACEhamiltonians.jl and version 2 of the data from Zenodo. I have moved the root folder of the data (ACEhamiltonians-data) inside the root of the ACEhamiltonians.jl. Based on recommendation I'm executing

$ julia --project=. test/model_recheck.jl
ERROR: LoadError: ACEbase.FIO.read_dict no implemented for symbol ACE2tb_OffModelWhole
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] read_dict(#unused#::Val{:ACE2tb_OffModelWhole}, D::Dict{String, Any})
   @ ACEbase.FIO ~/.julia/packages/ACEbase/OW7gD/src/fio.jl:72
 [3] read_dict(D::Dict{String, Any})
   @ ACEbase.FIO ~/.julia/packages/ACEbase/OW7gD/src/fio.jl:69
 [4] top-level scope
   @ ~/Dokumenter/Skole/phd/Liweipaper.git/test/model_recheck.jl:17
in expression starting at /home/marius/Dokumenter/Skole/phd/Liweipaper.git/test/model_recheck.jl:17

If you can say what to do for it to run without errors, it would be much appreciated.

jameskermode commented 1 year ago

@mcsloy should be able to advise on how best to do this with the latest version of the ACEhamiltonians code. As you say it should also be possible with the tagged version, maybe @zhanglw0521 can help you figure that out, for example by instantiating your project from his stored version of the Manifest.toml if you didn't already do that.

zhanglw0521 commented 1 year ago

If you are running exactly the same code on the arXiv.2111.13736 branch and still encountering this error, I would suggest the following:

If neither of them works, please update here with the latest error info so that we can look deeper into it.

computative commented 1 year ago

On the first remark, I have used

julia> ] 
(@v1.8) pkg> instantiate 

Is that what you suggest for updating the environment? If it is not, please advise

I have tried to paste and change those lines, but then I get a new error:

ERROR: LoadError: UndefVarError: rmse_total not defined
Stacktrace:
 [1] top-level scope
   @ ~/Dokumenter/Skole/phd/Liweipaper.git/test/model_recheck.jl:59
in expression starting at /home/marius/Dokumenter/Skole/phd/Liweipaper.git/test/model_recheck.jl:48
zhanglw0521 commented 1 year ago

I cannot reproduce this error actually and it does look strange to me. Would it because of the Julia version? I am using v1.65 and v1.7 and both of them look good.

Re updating the environment - would you please try

] up

and see what you get?

computative commented 1 year ago

I did the ] up. It changed a few things. Perhaps the error is related to how you checkout compared to me. I have tried two things (1) I have git checkout arXiv.2111.13736 which provides a file structure that does not contain the model_recheck file, so I wget it manually into the test-directory. (2) I have tried to git checkout 0bf9c5f406a285c3454b43452da9c20d294f18a7. In either case I get the same error amount the undefined variable.

zhanglw0521 commented 1 year ago

It is quite strange... How about manually overwriting your local Manifest.toml and src/dictionary.jl with the ones shown on git, running

] resolve

and then try again?

Edit: maybe better do a backup before overwriting

cortner commented 1 year ago

I think up is wrong here as it overwrites the Manifest. Setup the correct manifest and the just resolve. Don't up

zhanglw0521 commented 1 year ago

I think up is wrong here as it overwrites the Manifest. Setup the correct manifest and the just resolve. Don't up

Thanks! I will modify the above comments!

computative commented 1 year ago
(@v1.8) pkg> resolve
  No Changes to '~/.julia/environments/v1.8/Project.toml'
  No Changes to '~/.julia/environments/v1.8/Manifest.toml'

I can see there's detached head at arXiv.2111.13736. Can you say how you anticipate that I should receive the code from your repo? One option is to copy and paste the exact comments that I type into git to recover the correct code.

cortner commented 1 year ago

@zhanglw0521 -- is there multi-threading involved anywhere in this package? I just learned that our multi-threading has a significant bug from when Julia switched to a dynamic scheduler.

It is possible that this bug is visible on some systems but not others.

cortner commented 1 year ago

@computative -- technically it shouldn't matter whether you use Julia 1.8 or 1.7. But for debugging purposes can you please try to switch to 1.7 so that you use the same system as Liwei.

Secondly, I notice that you haven't actually activated the correct environment. You are working in the global Julia environment. If it isn't clear what I mean, then can you please read the Pkg docs; apologies if this was discussed before, I didn't have time to read the entire thread.

computative commented 1 year ago

I execute the code with julia --project=. test/model_recheck.jl

mcsloy commented 1 year ago

I can perhaps weigh in here. This is an error associated with loading and deserialising a JSON files. The code is complaining that it does not know how to deserialise a structure called “ACE2tb_OffModelWhole” as it can’t find an associated function named read_dict to perform the deserialisation operation. This is not an issue on your end this is an error present in the data. This is a holdover from the pervious version of the code and a time when the code was called “ACE2tb”. To correct for this find all instances of “ACE2tb_” in the offending JSON file(s) and replace them with with “ACEhamiltonians”. I wish there was a better solution.

TL;DR It’s not you, @computative; It’s the data. Search and replace “ACE2tb_” with “ACEhamiltonians” in the JSON files it tries to load.

computative commented 1 year ago

Thanks for your messages. So it sounds like I could be close to receiving a resolution. Is the code/data in the form that you want to release it? If you want to modify either, I'd be glad to hold on for a bit in case it should be changed in this way.

cortner commented 1 year ago

@mcsloy - thank you for chiming in.

Actually we can fix this via monkey-patching. Give me a minute

cortner commented 1 year ago

Here is an example : in the script where you are loading the model you can add something like the following line:

ACEhamiltonians.read_dict(::Val{:ACE2tb_TBModel}, D::Dict) = 
             ACEhamiltonians.read_dict(Val{:ACEhamiltonians_TBModel}(), D)

@mcsloy and @zhanglw0521 - this could go into the main code when you have time?

computative commented 1 year ago

@computative -- technically it shouldn't matter whether you use Julia 1.8 or 1.7. But for debugging purposes can you please try to switch to 1.7 so that you use the same system as Liwei.

Secondly, I notice that you haven't actually activated the correct environment. You are working in the global Julia environment. If it isn't clear what I mean, then can you please read the Pkg docs; apologies if this was discussed before, I didn't have time to read the entire thread.

I also receive this. I suspect it's not related to the issue:

(ACEhamiltonians) pkg> resolve
ERROR: Unsatisfiable requirements detected for package ForwardDiff [f6369f11]:
 ForwardDiff [f6369f11] log:
 ├─possible versions are: 0.9.0-0.10.34 or uninstalled
 └─restricted to versions 0.10.33 by an explicit requirement — no versions left
computative commented 1 year ago

@mcsloy - thank you for chiming in.

Actually we can fix this via monkey-patching. Give me a minute

Thanks. I will have a look.

cortner commented 1 year ago

Re ForwardDiff : this is odd - since 0.10.33 is in that range? Did you pin it? Can you send me your ] status please?

computative commented 1 year ago

(@v1.8) pkg> status Status ~/.julia/environments/v1.8/Project.toml [3e8ccfd2] ACE v0.12.44 [1e34e032] ACEatoms v0.0.12 [51974c44] ASE v0.5.4 [f67ccb44] HDF5 v0.16.12 [7073ff75] IJulia v1.23.3 [682c06a0] JSON v0.21.3 [945c410c] JuLIP v0.13.8 [438e738f] PyCall v1.94.1 [90137ffa] StaticArrays v1.5.11

(ACEhamiltonians) pkg> status Project ACEhamiltonians v0.0.1 Status ~/Dokumenter/Skole/phd/ACEhamiltonians.jl/Project.toml ⌅ [3e8ccfd2] ACE v0.12.22 ⌃ [1e34e032] ACEatoms v0.0.8 ⌅ [14bae519] ACEbase v0.2.2 [51974c44] ASE v0.5.4 [c7e460c6] ArgParse v1.1.4 → [7d9fca2a] Arpack v0.5.4 ⌅ [4fba245c] ArrayInterface v2.14.17 →⌃ [8e7c35d0] BlockArrays v0.16.21 →⌃ [13f3f980] CairoMakie v0.9.2 [aaf54ef3] DistributedArrays v0.6.6 [352459e4] ExtXYZ v0.1.8 → [5789e2e9] FileIO v1.16.0 [c27321d9] Glob v1.3.0 [f67ccb44] HDF5 v0.16.12 [42fd0dbc] IterativeSolvers v0.9.2 →⌃ [033835bb] JLD2 v0.4.26 [682c06a0] JSON v0.21.3 ⌃ [945c410c] JuLIP v0.13.7 [b964fa9f] LaTeXStrings v1.3.0 → [898213cb] LowRankApprox v0.5.2 [2fcf5ba9] NeighbourLists v0.5.5 [47be7bcc] ORCA v0.5.0 →⌃ [91a5bcdd] Plots v1.36.1 [438e738f] PyCall v1.94.1 → [d330b81b] PyPlot v2.11.0 [189a3867] Reexport v1.2.2 [f2b01f46] Roots v2.0.8 ⌃ [90137ffa] StaticArrays v1.5.9 [2913bbd2] StatsBase v0.33.21 [8ba89e20] Distributed [37e2e46d] LinearAlgebra [de0858da] Printf [9a3f8284] Random [10745b16] Statistics Info Packages marked with → are not downloaded, use instantiate to download Info Packages marked with ⌃ and ⌅ have new versions available, but those with ⌅ are restricted by compatibility constraints from upgrading. To see why use status --outdated

computative commented 1 year ago

No, I don't think so. I have not learnt about pinning, so I doubt I did that

cortner commented 1 year ago

I can't reproduce your problem. if I clone a fresh ACEhamiltonians, then

julia17 --project=. 
] instantiate

everything installs fine for me. I can't check the script right now, but at least this suggests that your version incompatibility must be because you've changed something in an unexpected way.

computative commented 1 year ago

ACEatoms also seems to frequenly fail precompilation on my machine. I am on GNU/Linux Debian 11 without systemd. Also my distribution of julia is straight from the horse's mouth, free of any hacking

zhanglw0521 commented 1 year ago

I can perhaps weigh in here. This is an error associated with loading and deserialising a JSON files. The code is complaining that it does not know how to deserialise a structure called “ACE2tb_OffModelWhole” as it can’t find an associated function named read_dict to perform the deserialisation operation. This is not an issue on your end this is an error present in the data. This is a holdover from the pervious version of the code and a time when the code was called “ACE2tb”. To correct for this find all instances of “ACE2tb_” in the offending JSON file(s) and replace them with with “ACEhamiltonians”. I wish there was a better solution.

Thank you Adam for looking into this issue. I've suggested above to add

Hmodel_whole = read_dict(JSON.parse(replace(read(Hmodelfile, String), "ACE2tb_"=>"ACEhamiltonians_")))

to the script but now Marius is having an even stranger error when running model_recheck.jl as mentioned here:

I have tried to paste and change those lines, but then I get a new error:

ERROR: LoadError: UndefVarError: rmse_total not defined
Stacktrace:
 [1] top-level scope
   @ ~/Dokumenter/Skole/phd/Liweipaper.git/test/model_recheck.jl:59
in expression starting at /home/marius/Dokumenter/Skole/phd/Liweipaper.git/test/model_recheck.jl:48
zhanglw0521 commented 1 year ago

Here is an example : in the script where you are loading the model you can add something like the following line:

ACEhamiltonians.read_dict(::Val{:ACE2tb_TBModel}, D::Dict) = 
             ACEhamiltonians.read_dict(Val{:ACEhamiltonians_TBModel}(), D)

@mcsloy and @zhanglw0521 - this could go into the main code when you have time?

Thanks. This looks a lot more concise than what I have done but I added some codes to the main code that did the same thing (cf. this line or the last three commits of the arXiv branch).

However, as Marius mentioned, he seems to have no access to the latest code on our arXiv.2111.13736 branch, e.g.,

I have tried two things (1) I have git checkout arXiv.2111.13736 which provides a file structure that does not contain the model_recheck file, so I wget it manually into the test-directory. (2) I have tried to git checkout 0bf9c5f406a285c3454b43452da9c20d294f18a7. In either case I get the same error amount the undefined variable.

So I am guessing that there might be something beyond the renaming issue?

computative commented 1 year ago

Here is an example : in the script where you are loading the model you can add something like the following line:

ACEhamiltonians.read_dict(::Val{:ACE2tb_TBModel}, D::Dict) = 
             ACEhamiltonians.read_dict(Val{:ACEhamiltonians_TBModel}(), D)

@mcsloy and @zhanglw0521 - this could go into the main code when you have time?

I have inserted the line above as the first line in model_recheck.jl after all the imports are finished (line 12). I did that in a fresh clone of ACEhamiltonians.jl. I am getting the same error as before (about the undefined variable rmse_total). Did I insert it in the wrong place?

Thanks for reading.

zhanglw0521 commented 1 year ago

This error is entirely irrelevant to the ACE2tb_TBModel vs ACEhamiltonians_TBModel stuff - i.e., you don't even need to add this line to model_recheck.jl and the renaming error had already been fixed when you used

Hmodel_whole = read_dict(JSON.parse(replace(read(Hmodelfile, String), "ACE2tb_"=>"ACEhamiltonians_")))

or else, you cannot even read our models successfully (that is to say, or else the code will complain before line 59, maybe as early as line 17).

The current error you get has nothing to do with the model-reading process. How about just adding a global before rmse_total inside the loop i.e., try changing line 59 to

global rmse_total += norm(E)^2

and see whether or not it works (I am not sure but maybe just worth a try)?

computative commented 1 year ago

I have inserted the correction you proposed.

ERROR: LoadError: UndefVarError: k not defined
Stacktrace:
 [1] top-level scope
   @ ~/Dokumenter/Skole/phd/Liwei.git/test/model_recheck.jl:60
in expression starting at /home/marius/Dokumenter/Skole/phd/Liwei.git/test/model_recheck.jl:48

What do you recommend now?

zhanglw0521 commented 1 year ago

The same - try

global k += 1
computative commented 1 year ago

Success