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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Dockerfile #456

Merged
merged 3 commits into from
Sep 27, 2022
Merged

Update Dockerfile #456

merged 3 commits into from
Sep 27, 2022

Conversation

moulai
Copy link
Contributor

@moulai moulai commented Sep 23, 2022

馃惁 Purpose of this PR 馃惁

The last Dockerfile update was two years ago. In the past two years, MAgPIE has undergone numerous updates and its reliance on the environment has also changed. This old version of the Dockerfile is currently unusable (can't successfully run MAgPIE after building and running the docker image).

Docker is the easiest way to run MAgPIE on a new machine. At the same time, it also enables the beginners of the MAgPIE to avoid the trouble of cumbersome environment configuration, and can concentrate on the learning and setting of the model itself.

I made an update to the Dockerfile. I have tested it to be able to run the default configuration and generate the validation.pdf successfully.

The updated details are as follows:

  1. Update the version of R to 4.2.1.
  2. Added missing software such as proj, gdal, git and some libs to the apt-get install list.
  3. Update the packages that R needs to install to be consistent with the latest instructions in README.md.
  4. Installed missing R packages, such as ncdf4, raster, etc.
  5. Replace the constants in the download and installation paths of GAMS with environment variables.
  6. Update the version of GAMS to 40.2.
    The version of GAMS can be easily changed by editing LATEST and LATEST_SHORT in the Dockerfile.
    As stated in README.md:

    "As the model benefits significantly of recent improvements in GAMS and CONOPT4 it is recommended to work with the most recent versions of both".

Note, as of September 2022, the latest GAMS version is 40.3, but this version has not been tested. It has been tested that MAgPIE works fine with GAMS 40.2.

@tscheypidi @giannou @pfuehrlich-pik

馃敡 Checklist for PR creator 馃敡

  • Low risk : Simple bugfixes (missing files, updated documentation, typos) or Start/output scripts
  • Medium risk : New realization / Changes to existing realization / Other changes which don't modify default.cfg
  • High risk : New input files (if cfg$input is changed in default.cfg) / Modification to core model (eg. changes in equations, calculations, introduction of new sets etc.) / Other changes in default.cfg
  • Providing additional information based on PR label
  • Low risk : No new model run needed.
  • Medium risk : Default run based on the current version of the fork from which PR is made
  • High risk
    • Default run from the current develop branch
    • Default run based on the current version of the fork from which PR is made
  • 馃搲 Performance loss/gain from current default behavior 馃搱

    • No performance loss/gain.
  • Added changes to CHANGELOG.md

    • I think modifying CHANGELOG.md is better done by the MAgPIE team.
    • Here is a possible change log for reference: **main** Update Dockerfile for running MAgPIE in a container
  • Compilation check (model starts without compilation errors - use gams main.gms action=c in model folder for testing).

  • No hard coded numbers and cluster/country/region names.

  • The new code doesn't contain declared but unused parameters or variables.

  • Where relevant, In-code comments added including documentation comments.

  • Made sure that documentation created with goxygen is okay (use goxygen::goxygen() for testing).

  • Changes to magpie4 R library for post processing of model output (ideally backward compatible).

    • No need.
  • Self-review of my own code.

  • In case of updated cellular input tgz file in default.cfg: scenario_config.csv has been updated accordingly (rcp1p9, rcp2p6 etc)

    • No need.

鈿狅笍 Additional notes or warnings 鈿狅笍

NA

馃毃 Checklist for RSE reviewer 馃毃

  • PR is labeled correctly.
  • CHANGELOG is updated correctly
  • No hard coded numbers and cluster/country/region names.
  • No unnecessary increase in module interfaces
  • All required updates in interfaces (if any) have been properly adressed in the module contracts
  • In-code comments and documentation comments are satisfactory.
  • model behavior/performance is satisfactory.
  • Requested changes (if any) were applied correctly

馃毃 Checklist for MAgPIE reviewer 馃毃

  • PR is labeled correctly.
  • CHANGELOG is updated correctly
  • No hard coded numbers and cluster/country/region names.
  • Changes to the model are scientifically sound
  • In-code comments and documentation comments are satisfactory.
  • model behavior/performance is satisfactory.
  • Requested changes (if any) were applied correctly

pfuehrlich-pik and others added 3 commits August 17, 2022 16:06
1. Update the version of R to 4.2.1.
2. Added missing software such as proj, gdal, git and some libs to the apt-get install list.
3. Update the packages that R needs to install to be consistent with the latest instructions in README.md.
4. Installed missing R packages, such as ncdf4, raster, etc.
5. Replace the constants in the download and installation paths of GAMS with environment variables.
6. Update the version of GAMS to 40.2. As stated in README.md, "as the model benefits significantly of recent improvements in GAMS and CONOPT4 it is recommended to work with the most recent versions of both".

Note, as of September 2022, the latest GAMS version is 40.3, but this version has not been tested. It has been tested that MAgPIE works fine with GAMS 40.2.
Copy link
Member

@tscheypidi tscheypidi 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. Thanks @moulai for fixing it! Much appreciated!

@tscheypidi tscheypidi changed the base branch from master to develop September 27, 2022 11:20
@tscheypidi
Copy link
Member

one note: we will merge that into the develop branch and it will be deployed to master with our next official release

@tscheypidi tscheypidi merged commit 722ac5e into magpiemodel:develop Sep 27, 2022
@moulai
Copy link
Contributor Author

moulai commented Sep 27, 2022

Thank you for your reply and approval!

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.

None yet

3 participants