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

{math}[foss/2022a] MXNet 1.9.1 w/ Python 3.10.4 #18251

Merged

Conversation

deniskristak
Copy link
Collaborator

@deniskristak deniskristak commented Jun 30, 2023

adding easyconfigs: MXNet-1.9.1-foss-2022a.eb and patches: mxnet-fix_blas.patch

this is a dependency for vscentrum/vsc-software-stack#89

@deniskristak deniskristak changed the title MXNet 1.9.1 {math}[foss/2022a] MXNet 1.9.1 w/ Python 3.10.4 Jun 30, 2023
@Micket Micket added the update label Jul 3, 2023
Micket
Micket previously requested changes Jul 7, 2023
Comment on lines 25 to 28
{
'download_filename': '5818c40f07bdb6307f9bc64e929836fe036da644.tar.gz',
'filename': 'mkldnn.tar.gz'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the 'source_urls' for this particular source dict and remove it from the gllobal source_urls.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Micket since there are a lot of different source urls, what is the benefit of letting easybuild find the correct combination of source url and source name rather than providing it all very specifically the way I tried to do? or is it just a thing of convention? i've fixed it according to your note, but I'm just interested

easybuild/easyconfigs/m/MXNet/MXNet-1.9.1-foss-2022a.eb Outdated Show resolved Hide resolved
easybuild/easyconfigs/m/MXNet/MXNet-1.9.1-foss-2022a.eb Outdated Show resolved Hide resolved
easybuild/easyconfigs/m/MXNet/MXNet-1.9.1-foss-2022a.eb Outdated Show resolved Hide resolved
easybuild/easyconfigs/m/MXNet/MXNet-1.9.1-foss-2022a.eb Outdated Show resolved Hide resolved
description = """Flexible and Efficient Library for Deep Learning"""

toolchain = {'name': 'foss', 'version': '2022a'}
toolchainopts = {'cstd': 'c++14', 'opt': True, 'pic': True, 'openmp': True, 'extra_cflags': '-lflexiblas'}
Copy link
Contributor

Choose a reason for hiding this comment

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

To be still need extra manual cflags here despite including the patch to their cmakelists to support flexiblas?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yep, won't build without it

Copy link
Contributor

Choose a reason for hiding this comment

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

Then i wonder what good

list(APPEND mshadow_LINKER_LIBS ${BLAS_LIBRARIES})

does in the patch. If anything, I think the easyblock should fix this, or the patch

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

you're right, this doesn't have to be in patch, removing...

Copy link
Contributor

Choose a reason for hiding this comment

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

well, i don't think there is any advantage to modifying the patch. i'm mostly curious as to where it fails

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Micket I think it's working now as is, but if you want, we could move the -lflexiblas option from extra_cflags in easyconfig to the easyblock somehow, however, I am not sure how to modify toolchainopts in easyblock...if you think it might be better that way, maybe drop me a hint about how to do that?

@boegel boegel added this to the 4.x milestone Aug 4, 2023
@boegel
Copy link
Member

boegel commented Aug 4, 2023

@boegelbot please test @ generoso
EB_ARGS="--include-easyblocks-from-pr 2955"

@boegelbot
Copy link
Collaborator

@boegel: Request for testing this PR well received on login1

PR test command 'EB_PR=18251 EB_ARGS="--include-easyblocks-from-pr 2955" EB_CONTAINER= /opt/software/slurm/bin/sbatch --job-name test_PR_18251 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 11376

Test results coming soon (I hope)...

- notification for comment with ID 1665337414 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

@deniskristak Unused patch mxnet-fix_blas.patch can be removed?

easybuild/easyconfigs/m/MXNet/MXNet-1.9.1-foss-2022a.eb Outdated Show resolved Hide resolved
toolchain = {'name': 'foss', 'version': '2022a'}
toolchainopts = {'cstd': 'c++14', 'opt': True, 'pic': True, 'openmp': True, 'extra_cflags': '-lflexiblas'}

# MXNet pulls in a bunch of submodules with specific commits
Copy link
Member

Choose a reason for hiding this comment

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

add comment to mention source for these commit IDs: https://github.com/apache/mxnet/tree/1.9.1/3rdparty

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2955
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
cns7 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/boegelbot/5d0a598be30a641cfc3e544c952080b6 for a full test report.

@easybuilders easybuilders deleted a comment from boegelbot Aug 17, 2023
@boegel
Copy link
Member

boegel commented Aug 17, 2023

@boegelbot please test @ jsc-zen2
EB_ARGS="--include-easyblocks-from-pr 2955"

@boegelbot
Copy link
Collaborator

@boegel: Request for testing this PR well received on jsczen2l1.int.jsc-zen2.easybuild-test.cluster

PR test command 'EB_PR=18251 EB_ARGS="--include-easyblocks-from-pr 2955" /opt/software/slurm/bin/sbatch --mem-per-cpu=4000M --job-name test_PR_18251 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen2.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 3184

Test results coming soon (I hope)...

- notification for comment with ID 1681797437 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegel
Copy link
Member

boegel commented Aug 17, 2023

Test report by @boegel
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2955
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3102.skitty.os - Linux RHEL 8.6, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/boegel/29e3e5efdf18fce024ef932c72572468 for a full test report.

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
Using easyblocks from PR(s) easybuilders/easybuild-easyblocks#2955
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
jsczen2c1.int.jsc-zen2.easybuild-test.cluster - Linux Rocky Linux 8.5, x86_64, AMD EPYC 7742 64-Core Processor (zen2), Python 3.6.8
See https://gist.github.com/boegelbot/f9dea5266365ebf06d21a5cb4dff1420 for a full test report.

@boegel boegel dismissed Micket’s stale review August 17, 2023 08:09

requested changes made

@boegel
Copy link
Member

boegel commented Aug 17, 2023

Going in, thanks @deniskristak!

@boegel boegel modified the milestones: 4.x, next release (4.8.1?) Aug 17, 2023
@boegel boegel merged commit b7ad622 into easybuilders:develop Aug 17, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants