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

Update CircleCI Python several versions for future proofing #223

Merged
merged 22 commits into from
Jan 9, 2024

Conversation

clairekope
Copy link
Contributor

Addresses #222

@gregbryan
Copy link
Contributor

Maybe try "-std=legacy" in Fortran compile?

@clairekope
Copy link
Contributor Author

That's certainly an easier flag. Though for some reason it's not being passed when ffte4X.F90 is getting compiled...

@gregbryan
Copy link
Contributor

The problem is that the answer testing works by compiling a version with the current gold-standard (which does not have this flag) and then compiling a version with this PR changes. The PR changes are clearly working because the test-compile-options test passes, but the first step of the test-suite (compilation of the current gold-standard) is the thing that is failing. There is no way to fix that in a PR except by updating the gold-standard. I think that is probably what we need to do, go from gold-standard-15 to gold-standard-16. Unfortunately, this takes a few steps which are outlined here: enzo-project/enzo-e#307

@clairekope
Copy link
Contributor Author

I did not realize that the gold standard version was also compiled. I'll create a new issue for tracking the gold standard update.

@clairekope clairekope requested a review from jwise77 October 10, 2023 16:22
@clairekope
Copy link
Contributor Author

clairekope commented Oct 10, 2023

I've pushed a lightweight tag as part of this PR that should switch the gold standard to building with -std=legacy (at least for the ubuntu machine file)

@gregbryan
Copy link
Contributor

Thanks Claire! Now it looks like it is failing due to a pynose problem. I'm guessing that you may need to bump dependencies-v3 to dependencies-v4 in config.yml to clear the dependencies cache, since you bumped the CircleCI image.

@clairekope
Copy link
Contributor Author

Tried Python 3.10.13, but nose might still not work (xlcnd/isbnlib#95) despite the issues of not(?) having python3 in the circleci image?

@clairekope
Copy link
Contributor Author

clairekope commented Dec 19, 2023

Path forward: return to the 3.11.5 image and migrate from nose to pytest. Nose hasn't been updated since 2015 anyway. Added as issue #229

@clairekope
Copy link
Contributor Author

Tests passed but there are unyt errors in the reference tests

@clairekope
Copy link
Contributor Author

12 of the reference tests fail due to a change with unyt v3. This is fixed by a PR on yt: yt-project/yt#4764

There is also an issue that, when comparing the new version of Enzo to the standard, the standard's answers don't seem to be saved.

@clairekope clairekope marked this pull request as ready for review January 8, 2024 20:55
@clairekope
Copy link
Contributor Author

clairekope commented Jan 8, 2024

I have figured out the nuances of CircleCI. For future reference:

  • when updating software versions for the test toolchain, make sure to update the dependency cache or you will get a vague error about a missing Python executable
  • when updating the gold standard, first update the CircleCI config to use this new standard: update the reference/answer tag as well as the name for the answer cache. Commit these changes to the config file and attach a git tag to this same commit. Placing a tag on an older commit than the one where the CircleCI config was updated will mean CircleCI will not cache answers correctly.

To maintainers:
Please please please squash these commits when merging.

Copy link
Contributor

@gregbryan gregbryan left a comment

Choose a reason for hiding this comment

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

Looks good to me -- thanks Claire for persevering though all of that!

@bwoshea
Copy link
Contributor

bwoshea commented Jan 9, 2024

Looks good to me!

Copy link
Contributor

@bwoshea bwoshea left a comment

Choose a reason for hiding this comment

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

Looks great, I'm approving and merging.

@bwoshea bwoshea merged commit 11ccaba into enzo-project:main Jan 9, 2024
4 checks passed
@clairekope clairekope deleted the circleci-python branch February 5, 2024 16:10
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.

3 participants