Skip to content

Conversation

@ben-c-2013
Copy link
Collaborator

No description provided.

@ben-c-2013 ben-c-2013 requested a review from kyrsjo January 28, 2026 15:13
@ben-c-2013 ben-c-2013 added the bug Something isn't working label Jan 28, 2026
@kyrsjo
Copy link
Collaborator

kyrsjo commented Jan 29, 2026

Not sure about this.

This seems to append "elegant" to CONFIG.elegant_exec. Looking into where this comes from, it can come from either (a) the TOML file directly https://github.com/abel-framework/ABEL/blob/main/abel/abelconfig.toml#L63 , (b) if elegant_use_container is False (i.e. laptop) then it takes just elegant_path https://github.com/abel-framework/ABEL/blob/main/abel/CONFIG.py#L127C1-L127C48 , and (c) if True (i.e. cluster) it uses a more complicated pattern https://github.com/abel-framework/ABEL/blob/main/abel/CONFIG.py#L124

Thus, the solution is probably to set the correct path in your TOML file, not this patch.

Also, I would generally strongly recommend to use os.path.join() to add together pieces of paths, not "+" and pray that the slashes are correct.

Copy link
Collaborator

@kyrsjo kyrsjo left a comment

Choose a reason for hiding this comment

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

See comment in main thread.

@ben-c-2013
Copy link
Collaborator Author

Not sure about this.

This seems to append "elegant" to CONFIG.elegant_exec. Looking into where this comes from, it can come from either (a) the TOML file directly https://github.com/abel-framework/ABEL/blob/main/abel/abelconfig.toml#L63 , (b) if elegant_use_container is False (i.e. laptop) then it takes just elegant_path https://github.com/abel-framework/ABEL/blob/main/abel/CONFIG.py#L127C1-L127C48 , and (c) if True (i.e. cluster) it uses a more complicated pattern https://github.com/abel-framework/ABEL/blob/main/abel/CONFIG.py#L124

Thus, the solution is probably to set the correct path in your TOML file, not this patch.

Also, I would generally strongly recommend to use os.path.join() to add together pieces of paths, not "+" and pray that the slashes are correct.

I totally agree with using os.path.join() intstead. A blunder from my side for not doing it.

However not sure if we should change the path in the TOML file at all, as the path is supposed to point to a folder containing a lot of the tools needed by ELEGANT, and not just the ELEGANT binary.

@kyrsjo
Copy link
Collaborator

kyrsjo commented Jan 29, 2026

However not sure if we should change the path in the TOML file at all, as the path is supposed to point to a folder containing a lot of the tools needed by ELEGANT, and not just the ELEGANT binary.

The TOML file comment says # Path to elegant executable., which would include the actual executable, not just the folder it is in. The confusion is probably because it works differently when using a container, in which case CONFIG.py adds the elegant.sif when generating elegant_exec and adding all the other incantations to actually launch elegant from inside this .sif.

So the code in elgant_wrapper was already correct, the problem was that the default in https://github.com/abel-framework/ABEL/blob/main/abel/abelconfig.toml#L59C57-L59C86 should have been ['$software_path', 'elegant', 'bin', 'elegant] I think. Alternatively, CONFIG.py should add elegant to elegant_path when generating elegant_exec.

@ben-c-2013
Copy link
Collaborator Author

Tested commits cd60d43 and 8b32db9 in the following way (tested both locally and on LUMI):

  • pytest -v tests/test_elegant_wrapper.py passed when the ELEGANT executable is located as specified by elegant_path.
  • Changed the folder name of the ELEGANT executable, pytest -v tests/test_elegant_wrapper.py skipped the tests as expected.
  • Changed the folder name of the ELEGANT executable, but with elegant_is_installed = true in .abelconfig.toml in my home directory. pytest -v tests/test_elegant_wrapper.py` now does not skip the tests and will fail them as expected.
  • Set elegant_is_installed = false in .abelconfig.toml in my home directory. pytest -v tests/test_elegant_wrapper.pynow will skip the tests even though the ELEGANT executable is located as specified byelegant_path`.

@ben-c-2013 ben-c-2013 merged commit 05dadc5 into main Feb 3, 2026
1 check passed
@ben-c-2013 ben-c-2013 deleted the bugfix__elegant_missing_test branch February 3, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants