Skip to content

Conversation

@nruneberg
Copy link
Contributor

Proposed changes

Added a page about VeloxChem on LUMI.
https://csc-guide-preview.2.rahtiapp.fi/origin/veloxchem/apps/veloxchem/

Checklist before requesting a review

@nruneberg nruneberg requested review from rkronberg and trossi June 6, 2025 08:27
Copy link

@trossi trossi left a comment

Choose a reason for hiding this comment

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

This link does not work: https://csc-guide-preview.2.rahtiapp.fi/origin/veloxchem/apps/veloxchem/ .

I left a few comments in the review too.


| Version | Available modules | Notes |
|:-------:|:------------------|:-----:|
| 1.0rc4 | veloxchem/cpu | CPU version |
Copy link

Choose a reason for hiding this comment

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

A few comments here:

  • Why to use release candidate instead of a stable version?
  • Which version does this correspond to? There is no tags/releases in https://github.com/VeloxChem/VeloxChem/tags.
  • I'd suggest to include version in the module name too (and git hash if there is no versioning)
  • I can't look into the modules as cannot open directory '/appl/local/csc/modulefiles/veloxchem/': Permission denied

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks Tuomas for the detailed and extended comments! Much appreciated! I'll address these points
as follows:

  • Preview disappeared: No idea why the preview is missing
  • Version choice: This is the version suggested by the VeloxChem developers. I couldn’t find explicit versioning or tags in the repo, so the only reference I could use is the output from "vlx --version" .
  • Module permissions: Thanks for pointing that out. Has now been fixed.

see and load the module. For example:

```bash
module use /appl/local/csc/modulefiles
Copy link

Choose a reason for hiding this comment

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

Suggested change
module use /appl/local/csc/modulefiles
module load Local-CSC

(Which one is recommended?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't even know about Local-CSC. Have only seen the other version, e.g. https://docs.csc.fi/apps/lammps/#lumi-c-hybrid-mpiopenmp

```bash
module use /appl/local/csc/modulefiles
module load veloxchem/cpu
source $VLXHOME/vlxenv/bin/activate
Copy link

Choose a reason for hiding this comment

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

I think it'd be better to install veloxchem outside venv (and set PYTHONPATHs in module file instead). Then users have possibility to create their own venv for a few extra packages if needed. (Another question is that would a container-based installation be better?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding this I have no opinion or experience. Maybe discuss it in detail outside this PR?

Comment on lines +97 to +99
export OMP_NUM_THREADS=8
export OMP_PLACES=cores
export SRUN_CPUS_PER_TASK=8
Copy link

Choose a reason for hiding this comment

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

Would it work to define #SBATCH --cpus-per-task=8 instead and make this block like in cpu run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right! That would be more consistent.

export SRUN_CPUS_PER_TASK=8

# Run VeloxChem
job=g-quad-neutral
Copy link

Choose a reason for hiding this comment

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

This input script could be included for testing.

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

Successfully merging this pull request may close these issues.

4 participants