Quantum-Accelerators / quacc

quacc is a flexible platform for computational materials science and quantum chemistry that is built for the big data era.
https://quantum-accelerators.github.io/quacc/
BSD 3-Clause "New" or "Revised" License
176 stars 48 forks source link

ORCA unit tests can apparently trigger the Ubuntu screen reader, which is also called `orca` #1091

Closed Andrew-S-Rosen closed 12 months ago

Andrew-S-Rosen commented 1 year ago

Environment

N/A

What is the issue?

This line is the effected issue: https://github.com/Quantum-Accelerators/quacc/blob/93b853b56cf46588e5859bdd4a8b5e0106bb4313/tests/core/recipes/compiled_recipes/test_real_orca_recipes.py#L9

When the user has Ubuntu orca screen reader installed, it tries to run the ORCA test suite because the above logic evalutes to True (even when the DFT code ORCA is not installed).

How can we reproduce the issue?

N/A

rishav-karanjit commented 12 months ago

Hi @Andrew-S-Rosen, there is two ways to solve this:

Which would you prefer? Also let me know if there is any typically path of version for ORCA. Can I also get assigned to this?

Andrew-S-Rosen commented 12 months ago

Hi @rishav-karanjit: Thanks for chiming in with your comments!

I see a few potential challenges with both approaches:

  1. Sadly, it can be installed anywhere, so we can't make any assumptions about it one way or another.

  2. This is a smart idea, although it involves actually running the executable, which is not ideal from a performance standpoint. Currently, the approach simply checks if orca is in PATH, which is basically a free operation.

Happy to take suggestions though! I don't assign issues. Anyone can submit a PR :)

That said, a recent upstream change in ASE broke all the code a few hours ago :/ Trying to patch that in #1141, so it probably makes sense to wait until I get that merged so the test suite actually runs appropriately. Sorry for the hassle there.

Andrew-S-Rosen commented 12 months ago

@rishav-karanjit: Alright, I fixed the issues mentioned in #1141. Tests now pass again if pulling from main.

rishav-karanjit commented 12 months ago

Thanks @Andrew-S-Rosen. If you have any approach you recommend to solve this?

Andrew-S-Rosen commented 12 months ago

@rishav-karanjit: Perhaps the easiest option is to take a modification to your first approach.

There's no way for us to know where the (quantum chemistry) ORCA package will be installed. However, there probably is a common place on Ubuntu where the screen reader ORCA is installed. I don't have an Ubuntu machine, however, so I can't run which orca to find out where that path is.

https://github.com/Quantum-Accelerators/quacc/blob/b038f11b2256124c752c8ba35397972dc556addf/tests/core/recipes/compiled_recipes/test_real_orca_recipes.py#L9

If we did, then what we could do is update this logic in the test suite to instead be something like:

from quacc import SETTINGS

orca_cmd = SETTINGS.ORCA_CMD # this is Path("orca") by default, hence the issue
has_orca = bool(orca_cmd.exists() and <orca_cmd is not in the Ubuntu screen reader place>)

Alternatively, one could do some hack to check the filesize of the orca executable instead of the location. The quantum chemistry ORCA is very large (300+ MB). I doubt the screen reader is that large, so one could have an intermediate cutoff that might be good enough.

The reason we don't want to check the version is that the quantum chemistry ORCA doesn't accept command-line arguments. Doing orca --version will actually try to start running a quantum chemistry calculation...

That said, I'm open to suggestions!

P.S. Since this is one of the only tests that don't run on GitHub actions, it will hard for you to confirm that your change works without downloading the DFT ORCA package from here. You don't need to do that, so if you propose a change, simply make a best effort judgment. I can test it locally since I have quantum chemistry ORCA downloaded on my machine.