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

Added test_traj; debloat #3

Merged
merged 28 commits into from
Jul 23, 2024
Merged

Added test_traj; debloat #3

merged 28 commits into from
Jul 23, 2024

Conversation

JHKru
Copy link
Member

@JHKru JHKru commented Jul 14, 2024

Added main tests for Trajectory objects.
Tests for unused formats were removed/commented out.
Tests for correct centering of trajectories were also removed (depends on MDTraj's RMSD
implementation; already present in main Biotite library).

Removed geometry-related functions from trajectory.py as well as ./geometry.

@JHKru JHKru changed the title Added test_traj Added test_traj; debloat Jul 14, 2024
@JHKru
Copy link
Member Author

JHKru commented Jul 15, 2024

An ARM runner image is seemingly assigned to our MacOS x86_64 job.

image

Is it possible, that we'll have to switch to MacOS 12/13 for these platforms
(see: actions/runner#3339 (comment))?

@JHKru
Copy link
Member Author

JHKru commented Jul 23, 2024

Tests are correctly run for MacOS-ARM, therefore this platform is no longer omitted from wheel-testing.
MacOS-x86-64 is run with an MacOS-ARM runner for macos-latest (this is also true for PRs in the main biotite repo).
Hence, macos-13 is now used in generate-wheels-matrix for x86-64.

.github/workflows/test_and_deploy.yml Outdated Show resolved Hide resolved
.github/workflows/test_and_deploy.yml Show resolved Hide resolved
Comment on lines +61 to +67
| jq -Rc '
if (contains("x86_64")) then
{"dist": ., "os": "macos-13"}
else
{"dist": ., "os": "macos-latest"}
end
' \
Copy link
Member

Choose a reason for hiding this comment

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

Nice idea! This is about fixing the performance issues in arm builds right?

Copy link
Member Author

@JHKru JHKru Jul 23, 2024

Choose a reason for hiding this comment

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

Oh, I haven't checked the performance on ARM yet.
It seems macos-latest + x86_64 is not supported and it always defaulted to macos-latest + ARM for that setting.
According to the compatibility table, we have to use macos-13 + x86_64 or macos-latest-large + x86_64:
https://github.com/actions/runner-images?tab=readme-ov-file#available-images

If there are no downsides for macos-latest-large, we could switch to the latter.

Copy link
Member Author

Choose a reason for hiding this comment

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

Large runners are seemingly only available for enterprise customers.
Let's keep macos-13, then.

@JHKru JHKru merged commit b23bbd7 into biotite-dev:main Jul 23, 2024
16 checks passed
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.

2 participants