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

Error reading 'extended' xyz format files #415

Open
venkatkapil24 opened this issue May 12, 2024 · 23 comments
Open

Error reading 'extended' xyz format files #415

venkatkapil24 opened this issue May 12, 2024 · 23 comments

Comments

@venkatkapil24
Copy link
Collaborator

Describe the bug
MACE returns error for an ".extxyz" file.

Traceback (most recent call last):
File "/home/ucapvka/source/mace/scripts/run_train.py", line 6, in
main()
File "/lustre/home/ucapvka/mace-venv/lib/python3.9/site-packages/mace/cli/run_train.py", line 157, in main
assert args.train_file.endswith(".xyz"), "Must specify atomic_numbers when using .h5 train_file input"
AssertionError: Must specify atomic_numbers when using .h5 train_file input

Typically XYZ files do not include anything other than positions/velovities. On the other hand, the extended XYZ in ASE/libatoms '.extxyz' is the standard format. I think MACE should include it and not assume .h5. Again, happy to open a quick PR.

@ilyes319
Copy link
Contributor

Hey @venkatkapil24, MACE can read extended xyz, but you need to rename the extension just .xyz.

@bernstei
Copy link
Collaborator

bernstei commented May 13, 2024 via email

@gabor1
Copy link
Collaborator

gabor1 commented May 13, 2024

I agree it should accept both .xyz and .extxyz

@bernstei
Copy link
Collaborator

I'm not even convinced it should check anything except, I guess, special-casing .h5. Anything else it could just pass to ase.io.read and let it deal with it automatically.

@venkatkapil24
Copy link
Collaborator Author

I'm not even convinced it should check anything except, I guess, special-casing .h5. Anything else it could just pass to ase.io.read and let it deal with it automatically.

I agree. This will also help read directly from json.

@bernstei
Copy link
Collaborator

I agree. This will also help read directly from json.

iff it's json that ase.io.read can parse

@stargolike
Copy link

today i upgrade my mace, and i meet this problem. it's not good idea, yesterday i can use extxyz and now i can't.

@ilyes319
Copy link
Contributor

just change your extension to .xyz for now.

@gabor1
Copy link
Collaborator

gabor1 commented Jun 11, 2024

ASE changes its behaviour randomly... we are aware of it.

@bernstei
Copy link
Collaborator

bernstei commented Jun 11, 2024

yesterday i can use extxyz and now i can't.

I'm actually very surprised that using extxyz isn't working. If anything, I'd expect ASE to have decided that plain xyz is not for extxyz.

@stargolike can you say precisely what versions of mace and ase you're trying to use? I just confirmed that the latest ase (3.23.0) reads .xyz and .extxyz suffix files with no problem.

@bernstei
Copy link
Collaborator

bernstei commented Jun 11, 2024

just change your extension to .xyz for now.

@ilyes319 Did you actually change something in MACE to cause this change?

@stargolike
Copy link

ASE changes its behaviour randomly... we are aware of it.

thanks, it's good method.

@stargolike
Copy link

ASE changes its behaviour randomly... we are aware of it.

it sounds so sad :(

@stargolike
Copy link

just change your extension to .xyz for now.

i solve the problem, thank you.

@stargolike
Copy link

just change your extension to .xyz for now.

@ilyes319 Did you actually change something in MACE to cause this change?

I newly reinstalled MACE, so I met this problem

@bernstei
Copy link
Collaborator

@ilyes319 Did you actually change something in MACE to cause this change?

I newly reinstalled MACE, so I met this problem

I was asking @ilyes319 if he intentionally changed anything in MACE to cause this. Because there's really no reason it should start happening, unless he hard-wired something, which isn't a great idea.

@bernstei
Copy link
Collaborator

@ilyes319 Did you actually change something in MACE to cause this change?

I newly reinstalled MACE, so I met this problem

I was asking @ilyes319 if he intentionally changed anything in MACE to cause this. Because there's really no reason it should start happening, unless he hard-wired something, which isn't a great idea.

Never mind - I see in the original post that started this thread that .xyz is hardwired. I stand by my statement that it should just defer to what ASE can read. A PR would be essentially trivial, but I don't know what branch to base it on. @ilyes319 ?

@stargolike
Copy link

stargolike commented Jun 12, 2024

@ilyes319 Did you actually change something in MACE to cause this change?

I newly reinstalled MACE, so I met this problem

I was asking @ilyes319 if he intentionally changed anything in MACE to cause this. Because there's really no reason it should start happening, unless he hard-wired something, which isn't a great idea.

i use

pip3 install torch torchvision torchaudio --extra-index-url https://download.pytorch.org/whl/cu116
git clone https://github.com/ACEsuit/mace.git
pip install ./mace

today i reinstall MACE in a new machine, and i think when i'm running this command, the lastest version ASE was installed.

@gabor1
Copy link
Collaborator

gabor1 commented Jun 12, 2024

We could temporarily pin ASE 3.22 like everyone else...

@bernstei
Copy link
Collaborator

bernstei commented Jun 12, 2024 via email

@bernstei
Copy link
Collaborator

OMFG. No. This has nothing to do with ASE's version. ".xyz" is hard-coded into MACE, for no good reason.

It needs to distinguish .h5 from things that are read with ase.io.read, but that's the only issue. It's easy to clean up, as soon as I know what branch to do it relative to.

@venkatkapil24
Copy link
Collaborator Author

Yes, this is a mace issue. I can only read .xyz or .h5.

@bernstei
Copy link
Collaborator

PR coming soon.

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

No branches or pull requests

5 participants