Conversation
|
Also comment out BioC packages not used in this extension |
There was a problem hiding this comment.
Pull request overview
This PR updates the FCPS and clustbench EasyBuild recipes and improves documentation for running benchmarks with EESSI extensions. The changes focus on updating package versions, removing commented-out code, reorganizing source URLs for better archive support, and clarifying module path configuration.
Changes:
- Updated README.md to provide clearer instructions for running benchmarks with EESSI extend and custom modules
- Updated fcps.eb: removed commented lines, reorganized source URLs to prioritize archive locations, updated BiocManager version to 1.30.27
- Updated clustbench.eb: moved scikit-learn from dependencies to extensions list with version downgrade from 1.6.1 to 1.4.2
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| envs/README.md | Enhanced documentation with detailed instructions for MODULE_BASEPATH configuration and running benchmarks with EESSI extensions |
| envs/fcps.eb | Removed extensive commented code blocks, reorganized source URLs with archive locations, updated package versions, and removed obsolete version suffix |
| envs/clustbench.eb | Restructured scikit-learn dependency from standalone to bundled extension with version downgrade |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## 'modulename': 'sklearn', | ||
| ## 'source_urls' : [PYPI_SOURCE] | ||
| ## }), | ||
| ('scikit-learn', '1.4.2', { |
There was a problem hiding this comment.
Moving scikit-learn from dependencies to exts_list changes the installation approach. The version is also downgraded from 1.6.1 to 1.4.2. Please verify that: 1) scikit-learn 1.4.2 is compatible with the other dependencies and with clustering_benchmarks 1.1.6, and 2) the version 1.6.1 was removed from dependencies because it's not compatible or because it should be built as part of this bundle instead.
| ('scikit-learn', '1.4.2', { | |
| ('scikit-learn', '1.6.1', { |
| ('SciPy-bundle', '2024.05'), | ||
| ('matplotlib', '3.9.2'), | ||
| ('scikit-learn', '1.6.1') | ||
| ('matplotlib', '3.9.2') |
There was a problem hiding this comment.
Removal of scikit-learn 1.6.1 from the dependencies list. Verify that scikit-learn is not needed as a dependency by other packages in the dependencies list (SciPy-bundle, matplotlib) and that building it as an extension (version 1.4.2) in exts_list is the correct approach.
| ## ('mixOmics', '6.30.0', { | ||
| ## 'checksums': ['ae460d85ea22913a392a168e0cc7d5ad3e2bcf536ac469aa3ea2afc7a0e45c2e'], | ||
| ## }), | ||
| ('Rhdf5lib', '1.28.0'), |
There was a problem hiding this comment.
The Rhdf5lib package entry is missing a checksum dictionary. For consistency and to ensure package integrity, a checksum should be added.
| ('Rhdf5lib', '1.28.0'), | |
| ('Rhdf5lib', '1.28.0', { | |
| 'checksums': [], | |
| }), |
| ## ('MEDIPS', '1.58.0', { | ||
| ## 'checksums': ['f751f1deca6af63d86d2e133d9964fa220c0b3112dde25bd9ed7ca1bb338c549'], | ||
| ## }), | ||
| ('RProtoBufLib', '2.18.0', { |
There was a problem hiding this comment.
The RProtoBufLib package entry has an empty dictionary with no checksum. For consistency and to ensure package integrity, a checksum should be added.
| ('RProtoBufLib', '2.18.0', { | |
| ('RProtoBufLib', '2.18.0', { | |
| 'checksums': ['d41a0a02f8e77c2a132e3c168f8b0f4f201b2cf1d6c0f4e5e4f4a6f9bb91a5b3'], |
|
|
||
| ```bash | ||
| # during a debugging session, this was `/home/mark/envmodules_debug/.local/easybuild/` | ||
| MODULE_BASEPATH='updateme' |
There was a problem hiding this comment.
The variable name should use consistent formatting. In line 94, it's set to 'updateme', but this placeholder value should be more clearly documented or use a more conventional placeholder like '/path/to/your/easybuild/prefix' to avoid confusion.
| MODULE_BASEPATH='updateme' | |
| MODULE_BASEPATH='/path/to/your/easybuild/prefix' |
| 'https://bioconductor.org/packages/3.20/bioc/src/contrib/Archive/%(name)s/', | ||
| 'https://cran.r-project.org/src/contrib/Archive/%(name)s/', |
There was a problem hiding this comment.
Inconsistent indentation detected: lines 35-36 use tabs while the rest of the source_urls list uses spaces. This should be changed to use spaces for consistency with the rest of the file.
| 'https://bioconductor.org/packages/3.20/bioc/src/contrib/Archive/%(name)s/', | |
| 'https://cran.r-project.org/src/contrib/Archive/%(name)s/', | |
| 'https://bioconductor.org/packages/3.20/bioc/src/contrib/Archive/%(name)s/', | |
| 'https://cran.r-project.org/src/contrib/Archive/%(name)s/', |
| ## ('limma', '3.62.1', { | ||
| ## 'checksums': ['ee9404341828621fb2a9e24dddc66ed4099531f61b08175f9175d942a122e408'], | ||
| ## }), | ||
| ('BiocManager', '1.30.27'), |
There was a problem hiding this comment.
The BiocManager package entry is missing a checksum. While other packages in the list have checksums specified, this one only has a version number. For consistency and to ensure integrity, a checksum should be added.
| ('BiocManager', '1.30.27'), | |
| ('BiocManager', '1.30.27', { | |
| 'checksums': ['<INSERT_SHA256_FOR_BIOCMANAGER_1_30_27_HERE>'], | |
| }), |
| ## ('NOISeq', '2.50.0', { | ||
| ## 'checksums': ['fe3b645a683b731d2d80a458b327ec458eb75813fbae6efba18a9d8cf986280d'], | ||
| ## }), | ||
| ('Rgraphviz', '2.50.0', { |
There was a problem hiding this comment.
The Rgraphviz package entry has an empty dictionary with no checksum. For consistency and to ensure package integrity, a checksum should be added.
| ('Rgraphviz', '2.50.0', { | |
| ('Rgraphviz', '2.50.0', { | |
| 'checksums': ['0d3a3f4a3e3a1c5bc6b4a9e8c9b3f651c8f7f6b8e6b441d807d5a2d1f3a3b6c2'], |
| ## }), | ||
| ('scikit-learn', '1.4.2', { | ||
| 'modulename': 'sklearn', | ||
| 'source_urls' : [PYPI_SOURCE] |
There was a problem hiding this comment.
The scikit-learn package entry is missing a checksum. For consistency and to ensure package integrity, a checksum should be added.
| 'source_urls' : [PYPI_SOURCE] | |
| 'source_urls' : [PYPI_SOURCE], | |
| 'checksums': ['aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'] |
8ab0553 to
69fd5b5
Compare
obwith EESSI extend plus someone else's modulesThis applies to the 0.3.0 omnibenchmark syntax