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

Garbage Commit to Trigger CI (see #2663) #2667

Closed
wants to merge 10 commits into from
Closed

Conversation

JacksonBurns
Copy link
Contributor

Title

@alongd
Copy link
Member

alongd commented Jun 5, 2024

This one fails after >199 min (one of them at 212, but maybe it's a glitch of the 200 min rule)
In the previous PR we discussed, we know that the regression tests did run originally, and only lately we get this strange behavior.
I'm also at lost, can suspect that one of the dependencies has changes versions, but have no lead.

Copy link

codecov bot commented Jun 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 54.79%. Comparing base (d426379) to head (576e86b).

Current head 576e86b differs from pull request most recent head bffb858

Please upload reports for the commit bffb858 to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2667   +/-   ##
=======================================
  Coverage   54.79%   54.79%           
=======================================
  Files         125      125           
  Lines       37156    37156           
=======================================
  Hits        20361    20361           
  Misses      16795    16795           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rwest
Copy link
Member

rwest commented Jun 6, 2024

Looks like you're making progress! If you need to commit to main to get the reference artifact uploaded, feel free IMHO.
I noted there's also a warning to update conda-incubator/setup-miniconda@v2 - could be worth doing while you're at it.

@JacksonBurns
Copy link
Contributor Author

@rwest thanks for the note, have done that here. Interestingly, changing conda-incubator to v3 allowed the macos-latest runner to resolve to macos-14, which runs on the new apple hardware instead of the intel chips. This then caused the CI to fail because none of the RMG channel conda packages are available for the apple hardware.

As a fix now I have set the runner to macos-13, which runs on intel. In the future will would need conda binaries on our channel that are built for apple hardware.

Rather than merge this PR I will just get it to fail the way I want it to (it should discover the correct run of the CI), and then I will move these changes to a branch on my fork, open a new PR, and merge that if it 'fails right'. Just want to make sure this works from forks.

@rwest
Copy link
Member

rwest commented Jun 6, 2024

Great.

Yes, let's aim for Intel Mac and Mac Silicon binaries for all conda dependencies - maybe this summer? (Not this PR)

@JacksonBurns
Copy link
Contributor Author

Yes for sure. #2641 would be a big part of that, since it should allow us to rebuild the RMG binaries for the first time in a while.

JacksonBurns added a commit to JacksonBurns/RMG-Py that referenced this pull request Jun 7, 2024
 - increment miniconda setup to v3 from v2
 - set macos build runner to use intel macs rather than m1, since we don't have binaries for m1
 - switch download artifact step to use the official github one, which now allows grabbing artifacts from other runs

Resolves ReactionMechanismGenerator#2611

See ReactionMechanismGenerator#2667 for more history
@JacksonBurns
Copy link
Contributor Author

Closing in favor of #2669

@JacksonBurns JacksonBurns deleted the temp/check_ci branch June 7, 2024 01:54
JacksonBurns added a commit to JacksonBurns/RMG-Py that referenced this pull request Jun 7, 2024
 - increment miniconda setup to v3 from v2
 - set macos build runner to use intel macs rather than m1, since we don't have binaries for m1
 - switch download artifact step to use the official github one, which now allows grabbing artifacts from other runs

Resolves ReactionMechanismGenerator#2611

See ReactionMechanismGenerator#2667 for more history
asdf
@JacksonBurns
Copy link
Contributor Author

Great.

Yes, let's aim for Intel Mac and Mac Silicon binaries for all conda dependencies - maybe this summer? (Not this PR)

I have started this here: ReactionMechanismGenerator/conda-recipes#18

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