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

ARM OSX Migrator #111

Merged

Conversation

regro-cf-autotick-bot
Copy link
Contributor

This feedstock is being rebuilt as part of the ARM OSX migration.

Feel free to merge the PR if CI is all green, but please don't close it
without reaching out the the ARM OSX team first at @conda-forge/help-osx-arm64.

If this PR was opened in error or needs to be updated please add the bot-rerun label to this PR. The bot will close this PR and schedule another one. If you do not have permissions to add this label, you can use the phrase @conda-forge-admin, please rerun bot in a PR comment to have the conda-forge-admin add it for you.

This PR was created by the regro-cf-autotick-bot. The regro-cf-autotick-bot is a service to automatically track the dependency graph, migrate packages, and propose package version updates for conda-forge. Feel free to drop us a line if there are any issues! This PR was generated by https://github.com/regro/cf-scripts/actions/runs/7539942352, please use this URL for debugging.

@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@YarShev
Copy link
Contributor

YarShev commented Jan 16, 2024

@conda-forge/help-osx-arm64, some of jobs failed. Could you help us here to make CI green?

recipe/meta.yaml Outdated
Comment on lines 29 to 31
build:
- python # [build_platform != target_platform]
- cross-python_{{ target_platform }} # [build_platform != target_platform]
Copy link
Member

Choose a reason for hiding this comment

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

You need to move this section to modin-core, i.e. all outputs that have a script and a python dependency should have this block.

Copy link
Contributor

Choose a reason for hiding this comment

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

Moved. One more question. Should we increase the build number? I guess we should have increased for unidist too.

Copy link
Member

Choose a reason for hiding this comment

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

You can but it is not required as the original architectures don't change and the only packages where this change applies are the ones for the new architecture.

Copy link
Contributor

@YarShev YarShev Jan 16, 2024

Choose a reason for hiding this comment

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

I see. Some jobs are still failed and throw RuntimeError: SHA256 mismatch. Do you know the reason?

Copy link
Member

Choose a reason for hiding this comment

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

This is happening for all jobs that build something. This means that the modin developer changed the release (tarball) retroactively.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is probably Ray specific and is not related to the changes in this PR.

Copy link
Member

Choose a reason for hiding this comment

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

build on Windows started failing for some reason due to OSError: [WinError 1455] The paging file is too small for this operation to complete. Do we put everything correctly in the meta.yml and around it?

This can happen when the build runs out of memory. It can (usually) be fixed by adding

azure:
  settings_win:
    variables:
      SET_PAGEFILE: "True"

to conda-forge.yml and then rerendering.

Copy link
Contributor

Choose a reason for hiding this comment

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

@h-vetinari, that is a good point. I will try in a separate PR. Do you know how much this configuration increases the pagefile size on Windows?

Copy link
Member

Choose a reason for hiding this comment

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

It's set to 8GB. I had an unmerged PR to make this configurable, but I don't think we've ever needed something else than 8GB.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. We have two PRs open to address this #115 and #114. Let's see if it helps to fix Modin on Ray testing.

Signed-off-by: Igoshev, Iaroslav <iaroslav.igoshev@intel.com>
Signed-off-by: Igoshev, Iaroslav <iaroslav.igoshev@intel.com>
YarShev
YarShev previously approved these changes Jan 16, 2024
Signed-off-by: Igoshev, Iaroslav <iaroslav.igoshev@intel.com>
@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

  • Failed to even lint the recipe, probably because of a conda-smithy bug 😢. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

Signed-off-by: Igoshev, Iaroslav <iaroslav.igoshev@intel.com>
@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@YarShev YarShev merged commit 3065ea0 into conda-forge:main Jan 16, 2024
18 checks passed
@YarShev YarShev mentioned this pull request Jan 16, 2024
Comment on lines +123 to +125
# TODO: Ray is failing on Windows when initializing, figure out the root cause.
# commands:
# - python -c "import modin.pandas as pd, modin.config as cfg; cfg.Engine.put('Ray'); df = pd.DataFrame([])"
Copy link
Member

Choose a reason for hiding this comment

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

You should figure out the root cause before merging, rather than unconditionally skipping tests (not even restricted to windows)...

Copy link
Contributor

Choose a reason for hiding this comment

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

In general, I agree with you but I decided to go ahead and merge this PR because of the following.

  1. Test for Modin on Ray started failing regardless of the changes in this PR. I verified this in Check CI for Ray #112.
  2. We didn't modify the current builds for Modin but rather just added builds for the new architecture.

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.

None yet

4 participants