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

{tools}[foss/2023a] pyiron v0.5.1 w/ Python 3.11.3 #19398

Merged
merged 112 commits into from
Jun 10, 2024

Conversation

lcniel
Copy link
Contributor

@lcniel lcniel commented Dec 11, 2023

(created using eb --new-pr)

ETA: I have limited time to work on this and did not expect it to be as involved as it turned out to be, it should ideally be split up and merged as separate PR:s. Currently my available time to work on splitting this up is very limited as software building is only one aspect of a position I currently work one day a week at.

@lcniel
Copy link
Contributor Author

lcniel commented Dec 11, 2023

Test report by @lcniel
FAILED
Build succeeded for 15 out of 16 (16 easyconfigs in total)
vera-skylake-build - Linux Rocky Linux 8.8, x86_64, Intel Xeon Processor (Skylake, IBRS, no TSX), Python 3.6.8
See https://gist.github.com/lcniel/d8ce2332150c15e3cf0b9c38f12cc0b2 for a full test report.

@lcniel
Copy link
Contributor Author

lcniel commented Dec 11, 2023

Test report by @lcniel
FAILED
Build succeeded for 37 out of 38 (16 easyconfigs in total)
vera-icelake-build - Linux Rocky Linux 8.8, x86_64, Intel(R) Xeon(R) Silver 4316 CPU @ 2.30GHz, Python 3.6.8
See https://gist.github.com/lcniel/5d2b6731071a2d2cd11dfdd67a4308e7 for a full test report.

@lcniel
Copy link
Contributor Author

lcniel commented Dec 11, 2023

Hmm, something broke when doing the from_pr build. Will have to check that later.

@verdurin
Copy link
Member

@lcniel did you really intend to include all the other PRs in addition to pyiron?
That normally indicates you didn't start from a clean checkout.

@lcniel
Copy link
Contributor Author

lcniel commented Dec 18, 2023

@lcniel did you really intend to include all the other PRs in addition to pyiron? That normally indicates you didn't start from a clean checkout.

I did, there's just a ton of dependencies that weren't updated upstream, with a lot of un-updated sub-dependencies... But this is becoming so complex that there's a risk that some of those dependencies will be covered by the time it's finished... I still haven't finished all the sub-sub dependencies on the plugins that turned out to be required 😭

EDIT: I saw now that there was an old, wrongly named pyiron easyconfig also included, that was not intentional, but all the others as far as I can see are right. It's just an insane dep tree.

@casparvl
Copy link
Contributor

@lcniel you can remove that erronesouly added easyconfig either from command line as described here or simply delete it in the GitHub web interface.

Also, like @verdurin signaled, it is unusual to have this many easyconfigs in one PR. Generally, I would recommend splitting it up to give the reviewer (like me :)) a bit of an easier job. You can then open a Pyiron PR that depends on other PRs by putting this in the opening post, like was done in e.g. this PR.

That way, we can process the dependencies in batches of 5 or so at a time. If there is some logical grouping (e.g. a Pyiron dep that itself requires 4 other deps), I'd bunch those together in one PR.

But this is becoming so complex that there's a risk that some of those dependencies will be covered by the time it's finished...

It also prevents this to some degree, since even if one of those dependencies has issues, at least the other PRs containing dependencies can already be merged. Finally, it also makes figuring out the CI issues a bit easier, as there are fewer changes per PR that can cause issues :)

Let me know what you prefer. I'm willing to give it a go to review this PR as a single entity (after you've fixed the CI issues) if you feel a bit overwhelmed by having to split it now. But splitting it may lead to a faster merge in the end... :)

@lcniel
Copy link
Contributor Author

lcniel commented Dec 21, 2023

Let me know what you prefer. I'm willing to give it a go to review this PR as a single entity (after you've fixed the CI issues) if you feel a bit overwhelmed by having to split it now. But splitting it may lead to a faster merge in the end... :)

Thanks a lot, then I will try to fix the CI issues. I only work with easybuild-related stuff 20%, so I don't have so much time to spend on this (basically the choice is between trying to fix it and abandoning it). I would never even have entertained taking this PR on if I had anticipated that it would grow like this and require this ridiculous amount of patching, or at least I would have done a much more modest toolchain/version move.

@lcniel you can remove that erronesouly added easyconfig either from command line as described here or simply delete it in the GitHub web interface.

Thanks, I thought I did but apparently I forgot to commit it. Not totally used to the Github interface, I'm more experienced with Gitlab.

@lcniel
Copy link
Contributor Author

lcniel commented Jun 10, 2024

Test report by @lcniel
SUCCESS
Build succeeded for 24 out of 24 (3 easyconfigs in total)
alvis10-11 - Linux Rocky Linux 8.9, x86_64, Intel(R) Xeon(R) Gold 6338 CPU @ 2.00GHz, Python 3.6.8
See https://gist.github.com/lcniel/42f41ccc7d658aaf4ec58e67bdd9318e for a full test report.

@ocaisa
Copy link
Member

ocaisa commented Jun 10, 2024

@boegelbot please test @ generoso

@easybuilders easybuilders deleted a comment from boegelbot Jun 10, 2024
@easybuilders easybuilders deleted a comment from boegelbot Jun 10, 2024
@ocaisa
Copy link
Member

ocaisa commented Jun 10, 2024

@boegelbot please test @ generoso

@boegelbot
Copy link
Collaborator

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

PR test command 'EB_PR=19398 EB_ARGS= EB_CONTAINER= EB_REPO=easybuild-easyconfigs /opt/software/slurm/bin/sbatch --job-name test_PR_19398 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

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

Test results coming soon (I hope)...

- notification for comment with ID 2158066816 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).

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
cns1 - Linux Rocky Linux 8.9, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/boegelbot/600a0344784275deefea1a1ce94e81da for a full test report.

@easybuilders easybuilders deleted a comment from boegelbot Jun 10, 2024
@easybuilders easybuilders deleted a comment from boegelbot Jun 10, 2024
@ocaisa
Copy link
Member

ocaisa commented Jun 10, 2024

@boegelbot please test @ jsc-zen3

@boegelbot
Copy link
Collaborator

@ocaisa: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de

PR test command 'if [[ develop != 'develop' ]]; then EB_BRANCH=develop ./easybuild_develop.sh 2> /dev/null 1>&2; EB_PREFIX=/home/boegelbot/easybuild/develop source init_env_easybuild_develop.sh; fi; EB_PR=19398 EB_ARGS= EB_CONTAINER= EB_REPO=easybuild-easyconfigs EB_BRANCH=develop /opt/software/slurm/bin/sbatch --job-name test_PR_19398 --ntasks=8 ~/boegelbot/eb_from_pr_upload_jsc-zen3.sh' executed!

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

Test results coming soon (I hope)...

- notification for comment with ID 2158106746 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).

@boegelbot
Copy link
Collaborator

Test report by @boegelbot
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
jsczen3c1.int.jsc-zen3.fz-juelich.de - Linux Rocky Linux 9.4, x86_64, AMD EPYC-Milan Processor (zen3), Python 3.9.18
See https://gist.github.com/boegelbot/a6f7ba64a9202c911b896f3bdad93ac1 for a full test report.

Copy link
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

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

@lcniel The merging of this PR is the EasyBuild gift to you for finishing your PhD 😛

@ocaisa ocaisa merged commit f19c2c0 into easybuilders:develop Jun 10, 2024
9 checks passed
@Flamefire
Copy link
Contributor

Test report by @Flamefire
SUCCESS
Build succeeded for 3 out of 3 (3 easyconfigs in total)
n1455 - Linux RHEL 8.7 (Ootpa), x86_64, Intel(R) Xeon(R) Platinum 8470 (icelake), Python 3.8.13
See https://gist.github.com/Flamefire/60d5b5bcec35a2334eee76d47de9168a for a full test report.

@boegel boegel modified the milestones: 4.x, 4.9.2 Jun 10, 2024
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.

7 participants