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.

Sign up for GitHub

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

Jump to bottom

CI/setup #9

Merged
merged 32 commits into from
Jan 22, 2024
Merged

CI/setup #9

merged 32 commits into from
Jan 22, 2024

Conversation

lllAlexanderlll
Copy link
Contributor

lllAlexanderlll and others added 19 commits January 15, 2024 10:14
@le1nux le1nux self-requested a review January 17, 2024 16:37
le1nux
le1nux requested changes Jan 17, 2024
View reviewed changes
Copy link
Member

@le1nux le1nux left a comment

Choose a reason for hiding this comment

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

Apart from my comments, this looks good to me! I mainly covered the code part.
@luzian-hahn could you take a look at the GitHub workflows and docs generation, please?

config_files/config_lorem_ipsum.yaml Show resolved Hide resolved
docs/source/configuration.rst Show resolved Hide resolved
docs/source/index.rst Outdated Show resolved Hide resolved
tests/dataloader/test_packed_dataset.py Outdated Show resolved Hide resolved
@lhahn-iis
refactor: rename loop var more meaningfully
201aff3
@lhahn-iis
test: use cpu-supported setup with lorem_ipsum
41cecab 
Since the github runner is cpu-only currently,
 the tests should work there.
If we want to verify GPU-Features,
 we should prepare a github runner respectively
@lhahn-iis
Copy link
Contributor

I fixed the tests for a CPU-only environment and tested them once again here in this PR.
Unfortunately I was only able to do so, by deactivating GPU-only features within the lorem_ipsum test scenario. If we want our CI-unittests to verify a working setup with GPUs, we might want to add another runner with GPUs to have the tests executed on it.

@lhahn-iis lhahn-iis mentioned this pull request Jan 22, 2024
Closed
@le1nux
Copy link
Member

le1nux commented Jan 22, 2024

LGTM :) Thanks for addressing the remarks.

@le1nux le1nux added the enhancement New feature or request label Jan 22, 2024
@le1nux le1nux merged commit ea2e2f9 into main Jan 22, 2024
@lllAlexanderlll lllAlexanderlll deleted the ci/setup branch February 21, 2024 12:23
le1nux added a commit that referenced this pull request Mar 13, 2024
@le1nux
Merge pull request #9 from Modalities/ci/setup
c5fd10c 
CI/setup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@le1nux le1nux le1nux requested changes

@lhahn-iis lhahn-iis lhahn-iis left review comments

@mali-git mali-git Awaiting requested review from mali-git

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lllAlexanderlll @lhahn-iis @le1nux