Skip to content

Sbra 357 propose dockerfile#25

Merged
rmslm merged 8 commits into
mainfrom
SBRA-357-propose-dockerfile
May 16, 2022
Merged

Sbra 357 propose dockerfile#25
rmslm merged 8 commits into
mainfrom
SBRA-357-propose-dockerfile

Conversation

@rmslm
Copy link
Copy Markdown
Contributor

@rmslm rmslm commented Apr 20, 2022

Create a docker file corresponding to the ai4sim env

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2022

Coverage report for weather_forecast/gwd/ use-case

File Coverage Missing
All files 94%
data.py 98% 157
models.py 89% 37-40 90 99
trainer.py 88% 75-76 80

Minimum allowed coverage is 80%

Generated by 🐒 cobertura-action against 5375c11

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2022

Coverage report for combustion/unets/ use-case

File Coverage Missing
All files 91%
data.py 91% 59 70 72 113 123 129
models.py 87% 41-42 45 48
trainer.py 75% 59-62 66-67 71
unet.py 96% 83 89

Minimum allowed coverage is 80%

Generated by 🐒 cobertura-action against 5375c11

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 20, 2022

Coverage report for combustion/gnns/ use-case

File Coverage Missing
All files 82%
config.py 68% 74-77 81 85-90 94-101
data.py 81% 109-136 162 216
inferer.py 89% 224-240
models.py 73% 85 116-131 156-160 210-214
plotters.py 93% 55-56 142-143 146-147 160 165
trainer.py 68% 67-72 81-83 87-89

Minimum allowed coverage is 80%

Generated by 🐒 cobertura-action against 5375c11

@rdruilhe rdruilhe requested review from A669015 and rdruilhe May 3, 2022 08:56
Copy link
Copy Markdown
Contributor

@rdruilhe rdruilhe left a comment

Choose a reason for hiding this comment

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

All the Dockerfiles seems to be lore or less the same. Why don't you create a single dockerfile with the use case as arguments. It will avoid maintaining multiple files that are almost identical.

Copy link
Copy Markdown
Contributor

@rdruilhe rdruilhe left a comment

Choose a reason for hiding this comment

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

All the Dockerfiles seems to be lore or less the same. Why don't you create a single dockerfile with the use case as arguments. It will avoid maintaining multiple files that are almost identical.

Copy link
Copy Markdown
Contributor

@rdruilhe rdruilhe left a comment

Choose a reason for hiding this comment

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

Each folder is independent from the others so that there is replication of code.

Copy link
Copy Markdown
Contributor

@leonicoletti leonicoletti left a comment

Choose a reason for hiding this comment

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

Looks good to me

Comment thread combustion/gnns/Dockerfile Outdated
WORKDIR /home/ai4sim

# clone ai4sim code
RUN git clone https://github.com/AI4SIM/model-collection.git
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Clone the full repo in the image seem a bit overkill. Only the source of the use case could be embeded.
That could be done with a first temporary image used to retrive the source, and the final one will just copy the part it needs.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The docker image built in our internal repo are good example of the method.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The idea was to use a donwloader image (as done here) for all download/git clone/... stuffs.
Then only the part of relevant code (i.e. the use case related code) can be copied in the final image.
Please, note, it will require to remove the generic nox file at the /tools/nox path to copy/paste its content in the nox_file.py in each use-case, such that the divergence of each use-case will not break the nox sessions.

Comment thread combustion/gnns/Dockerfile Outdated
WORKDIR /home/ai4sim

# clone ai4sim code
RUN git clone https://github.com/AI4SIM/model-collection.git
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The docker image built in our internal repo are good example of the method.

Copy link
Copy Markdown
Contributor

@rdruilhe rdruilhe left a comment

Choose a reason for hiding this comment

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

To avoid downloading the whole git repository, Lionel suggest to create a multistage Dockerfile with a first step including an image dedicated to download the files. Then, in the final image, a copy of the correct subfolders is made between the downloader and image. I agree with this way of doing stuffs.

@rmslm rmslm merged commit bab0d08 into main May 16, 2022
@rmslm rmslm deleted the SBRA-357-propose-dockerfile branch May 16, 2022 12:31
ai4sim-pr-app Bot pushed a commit that referenced this pull request Apr 14, 2026
Bumps the uv group with 2 updates in the /weather-forecast/nwp-emulation/panguweather directory: [pillow](https://github.com/python-pillow/Pillow) and [pytest](https://github.com/pytest-dev/pytest).
Bumps the uv group with 2 updates in the /reactive-flows/cnf-combustion/gnns directory: [pillow](https://github.com/python-pillow/Pillow) and [pytest](https://github.com/pytest-dev/pytest).
Bumps the uv group with 1 update in the /reactive-flows/cnf-combustion/unets directory: [pytest](https://github.com/pytest-dev/pytest).
Bumps the uv group with 2 updates in the /weather-forecast/ecrad-3d-correction/unets directory: [pillow](https://github.com/python-pillow/Pillow) and [pytest](https://github.com/pytest-dev/pytest).
Bumps the uv group with 1 update in the /weather-forecast/gravity-wave-drag/cnns directory: [pytest](https://github.com/pytest-dev/pytest).


Updates `pillow` from 12.1.1 to 12.2.0
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@12.1.1...12.2.0)

Updates `pytest` from 9.0.2 to 9.0.3
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@9.0.2...9.0.3)

Updates `pillow` from 12.1.1 to 12.2.0
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@12.1.1...12.2.0)

Updates `pytest` from 9.0.2 to 9.0.3
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@9.0.2...9.0.3)

Updates `pytest` from 9.0.2 to 9.0.3
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@9.0.2...9.0.3)

Updates `pillow` from 12.1.1 to 12.2.0
- [Release notes](https://github.com/python-pillow/Pillow/releases)
- [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst)
- [Commits](python-pillow/Pillow@12.1.1...12.2.0)

Updates `pytest` from 9.0.2 to 9.0.3
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@9.0.2...9.0.3)

Updates `pytest` from 9.0.2 to 9.0.3
- [Release notes](https://github.com/pytest-dev/pytest/releases)
- [Changelog](https://github.com/pytest-dev/pytest/blob/main/CHANGELOG.rst)
- [Commits](pytest-dev/pytest@9.0.2...9.0.3)

---
updated-dependencies:
- dependency-name: pillow
  dependency-version: 12.2.0
  dependency-type: indirect
  dependency-group: uv
- dependency-name: pytest
  dependency-version: 9.0.3
  dependency-type: indirect
  dependency-group: uv
- dependency-name: pillow
  dependency-version: 12.2.0
  dependency-type: indirect
  dependency-group: uv
- dependency-name: pytest
  dependency-version: 9.0.3
  dependency-type: indirect
  dependency-group: uv
- dependency-name: pytest
  dependency-version: 9.0.3
  dependency-type: indirect
  dependency-group: uv
- dependency-name: pillow
  dependency-version: 12.2.0
  dependency-type: indirect
  dependency-group: uv
- dependency-name: pytest
  dependency-version: 9.0.3
  dependency-type: indirect
  dependency-group: uv
- dependency-name: pytest
  dependency-version: 9.0.3
  dependency-type: indirect
  dependency-group: uv
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

4 participants