Skip to content

Add dev container#86

Merged
pelesh merged 11 commits intodevelopfrom
add-dev-container
May 30, 2025
Merged

Add dev container#86
pelesh merged 11 commits intodevelopfrom
add-dev-container

Conversation

@alexander-novo
Copy link
Collaborator

Description

Adds a dev container to simplify onboarding of new developers who aren't familiar with installing C++ dependencies, or who work in an environment like Windows with complex build processes. Developers using VS Code can simply build the included container, and re-open their workspace inside the container, which will mount the project directory into an environment with all of GridKit's prerequisites pre-installed.

Further comments

In the future, this container could be modified to be used as a deployment environment for GridKit. This container is currently sitting at ~677 MB (which is mostly software like cmake, g++, etc.) but builds in < 1 minute for me.

@alexander-novo alexander-novo requested a review from pelesh April 22, 2025 15:19
@pelesh pelesh requested a review from superwhiskers April 30, 2025 14:25
@pelesh pelesh added development Features/Tools related to development of GridKit, rather than use as a library. good first issue Good for newcomers labels Apr 30, 2025
Copy link
Collaborator

@superwhiskers superwhiskers left a comment

Choose a reason for hiding this comment

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

i've successfully built the container image and built gridkit in a running container with every test passing using podman (not in vscode, without docker).

my only comment would be that you could make the image smaller if you used an alpine linux base image instead of ubuntu. doing this is entirely up to you, but it could be worth it.

@superwhiskers
Copy link
Collaborator

here's a dockerfile which builds one with alpine. it's around 417mb. the unfamiliarity of the environment may be a reason to not use it but i thought i'd provide it anyway as a suggestion. it's up to you

FROM alpine:latest AS builder

RUN apk update && apk add --no-cache gcc g++ cmake samurai git openblas-dev
RUN git clone --single-branch --depth 1 --branch v7.10.2 https://github.com/DrTimothyAldenDavis/SuiteSparse.git /tmp/suitesparse && \
    cd /tmp/suitesparse && \
    mkdir -p build && cd build && \
    cmake -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DSUITESPARSE_ENABLE_PROJECTS="klu" .. && \
    ninja install && \
    rm -rf /tmp/suitesparse
# temporary switch to develop to handle LLNL/sundials#704
RUN git clone --single-branch --depth 1 --branch develop https://github.com/LLNL/sundials.git /tmp/sundials && \
    cd /tmp/sundials && \
    mkdir -p build && cd build && \
    cmake -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DENABLE_KLU=on .. && \
    ninja install && \
    rm -rf /tmp/sundials

FROM alpine:latest

RUN apk update && apk add --no-cache gcc g++ cmake samurai git openblas
COPY --from=builder /usr/local /usr/local
COPY --from=builder /usr/include /usr/include

@alexander-novo
Copy link
Collaborator Author

alexander-novo commented May 28, 2025

I like the idea, but since the point was to make it as easy as possible to setup an environment for power systems people (several of whom I've talked to and aren't familiar with Linux), I think Ubuntu is probably the best option. My image is 677 MB - I don't know if the space savings is worth it?

I'd also prefer if we opened an issue to update the dockerfile with a new tagged version of Sundials once they release a new version with the fix, rather than downloading develop?

@superwhiskers
Copy link
Collaborator

develop was necessary for the tests to pass. see the issue linked in the comment as well as @pelesh's comment here

@alexander-novo
Copy link
Collaborator Author

Well we're officially advertising the prerequisite version of SUNDIALS >= 7.0.0 in the README, so that either needs to be adjusted or that test needs to be disabled.

Regardless develop isn't an appropriate dependency version requirement - it should be pinned to the specific commit that fixes the issue. I have no clue how clean the develop branch in SUNDIALS is meant to be, and any random commit could just start breaking docker images.

@pelesh
Copy link
Collaborator

pelesh commented May 28, 2025

Well we're officially advertising the prerequisite version of SUNDIALS >= 7.0.0 in the README, so that either needs to be adjusted or that test needs to be disabled.

We need to change this and set new depencency to SUNDIALS develop branch > 07d21c2c5ae33211a9a2fafd8eac56de5582dce0

This will likely be included in SUNDIALS release 7.3.1.

I suggest you just update the main README file and we can merge this.

Copy link
Collaborator

@superwhiskers superwhiskers left a comment

Choose a reason for hiding this comment

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

some minor suggestions i found while writing the alpine dockerfile

Copy link
Collaborator

@superwhiskers superwhiskers left a comment

Choose a reason for hiding this comment

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

i don't have anything else beyond this. i think this is good to merge as-is, but this one suggestion would reduce the size of the image a little bit more.

Don't install "recommended" packages through `apt`. Also install `make` since it is recommended by `cmake`.
@pelesh
Copy link
Collaborator

pelesh commented May 30, 2025

We should probably use Spack to build GridKit within the container, but let's merge this first.

CC @nkoukpaizan

@pelesh pelesh merged commit 7d4621e into develop May 30, 2025
3 checks passed
@superwhiskers
Copy link
Collaborator

@nkoukpaizan, the make package was not strictly necessary to install, we have ninja already. probably the only instance in which it is necessary is if an end user using the container to develop something wanted to generate makefiles instead of a build configuration for ninja.

@alexander-novo
Copy link
Collaborator Author

Yes that was precisely why I included it. This is a dev container so I wanted it to be a bit “batteries included”. In the future when we design a production containers, moving to alpine and stripping build tools like ninja and make will be good.

@pelesh pelesh deleted the add-dev-container branch June 10, 2025 18:02
WiktoriaZielinskaORNL pushed a commit that referenced this pull request Jul 23, 2025
* Add dev container

* Update contributing.md with container info

* Reduce container size a bit

Don't install "recommended" packages through `apt`. Also install `make` since it is recommended by `cmake`.

---------

Co-authored-by: alexander-novo <alexander-novo@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development Features/Tools related to development of GridKit, rather than use as a library. good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants