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

Add extented linux CI #184

Closed
wants to merge 1 commit into from
Closed

Add extented linux CI #184

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 1, 2021

@tjhei, this is probably related to #83, since this PR adds an extended CI on linux machines to test some different set of external packages. As the build/src/unpack directories are all removed instantaneously after package installation the memory of the virtual machines should be sufficient.

@tjhei
Copy link
Member

tjhei commented Jun 3, 2021

First, adding some additional configurations is a good idea. I would like to avoid running for 2+ hours for each commit, though. That means we should try to be a bit conservative in what we test.
Would you mind explaining why you add a second .yml file while ignoring the already existing main.yml? If you want to split them by OS, please do (but then remove linux from main.yml)


jobs:
ubuntu18-minimal:
name: ubuntu 18.04 minimal
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 not exactly minimal, is it?

echo 'BUILD_EXAMPLES=OFF' >> candi.cfg

# dealii version
echo 'DEAL_II_VERSION=v9.3.0-rc1' >> candi.cfg
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 much better to be deleted, as we will automatically test the current stable version then.

Copy link
Author

Choose a reason for hiding this comment

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

This was a relict of the draft, sorry. At this point I would suggest to switch the dealii version to master on the candi-master branch, as it looks like each dealii-version has its own candi-branch?!

run: |
sudo apt-get update
sudo apt-get install \
doxygen graphviz \
Copy link
Member

Choose a reason for hiding this comment

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

why is doxygen and graphviz needed?

Copy link
Author

Choose a reason for hiding this comment

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

This was for building the documentation (Future option I planned)

.github/workflows/linux-build.yml Show resolved Hide resolved
./candi.sh -j 2 -y --packages="once:cmake \
once:petsc \
once:slepc \
once:sundials \
Copy link
Member

Choose a reason for hiding this comment

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

what is the motivation for this selection of packages?

mpi-default-dev \
libboost-all-dev libsuitesparse-dev

# Install cmake via candi to hit the version 3.16.3
Copy link
Member

Choose a reason for hiding this comment

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

why not use the system package?

Copy link
Author

Choose a reason for hiding this comment

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

This was due to a bug with cmake 3.20+, which is the default on the virtual machines. However, I will test it again.

echo 'BUILD_EXAMPLES=OFF' >> candi.cfg

# dealii version
echo 'DEAL_II_VERSION=v9.3.0-rc1' >> candi.cfg
Copy link
Member

Choose a reason for hiding this comment

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

remove

mpi-default-dev \
libboost-all-dev libsuitesparse-dev

# Install cmake via candi to hit the version 3.16.3
Copy link
Member

Choose a reason for hiding this comment

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

why?

once:adolc \
once:arpack-ng \
once:assimp \
once:gmsh \
Copy link
Member

Choose a reason for hiding this comment

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

it seems gmsh is not compiling correctly. This is worth checking out...

Copy link
Author

Choose a reason for hiding this comment

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

An update of the gmsh version fixes this. I will prepare this soon.

once:slepc \
once:sundials \
once:superlu_dist \
once:trilinos \
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation for "full" and trilinos separately?

@ghost
Copy link
Author

ghost commented Jun 17, 2021

This PR was meant more as a draft, better would have been an issue at this state.

I can understand that tests longer than 2 hours are annoying. However, I thought that here we have a good opportunity to test all external dependencies. Also, I think that the frequency of code changes is much lower than for example in dealii/dealii.

I will provide an update in the coming weeks.

@ghost ghost mentioned this pull request Jun 18, 2021
23 tasks
@ghost ghost marked this pull request as draft June 18, 2021 13:12
@ghost
Copy link
Author

ghost commented Jun 21, 2021

@tjhei, as it seems that you are just online. Should we discuss this pr per video call?

@tjhei
Copy link
Member

tjhei commented Jun 21, 2021

Not in the next few days, sorry.

@ghost
Copy link
Author

ghost commented Jun 21, 2021

No problem, maybe let me know if you have time.

@ghost ghost closed this Dec 1, 2022
@ghost ghost deleted the ci_linux branch December 1, 2022 14:24
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants