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 carbon emissions admin configuration options #16307

Conversation

Renni771
Copy link
Contributor

@Renni771 Renni771 commented Jun 24, 2023

Addresses #15046. This feature branch adds admin config parameters and admin documentation for carbon emissions reporting. The following admin flags were added:

  • geographical_server_location_code an ISO 3166 code specifying the geographical location of the galaxy instance.
  • power_usage_effectiveness the PUE value to use in carbon emissions calculations.
  • carbon_emission_estimates a feature toggle flag so reporting can be completely disabled when needed.

The following flags are automatically set based on the value of geographical_server_location_code:

  • geographical_server_location_name the actual name corresponding to geographical_server_location_code.
  • carbon_intensity the carbon intensity value corresponding to the configured location.

The location is picked from a list of supported locations in ISO 3166 format which is documented over here. This standard allows users to specify both countries and regions within larger countries. If no location is set or the configured location is invalid or unsupported, the config logic defaults to global values. The UI is dynamic and will display the location name, carbon intensity and PUE values. If global values are used, the UI reflects this too. Here's what the UI looks like:

Screenshot from 2023-06-24 13-48-32

Here's a list of further additions:

  • Documentation for how to configure the carbon emissions feature has been added to doc/source/admin/carbon_emissions.md.
  • The docs and config files all point to where information on where supported locations can be found. So, instead of specifying the format, we just refer admins to the list of supported values.
  • The docs include a link to resources on how to calculate PUE.
  • When geographical_server_location_code is invalid or unsupported, the global default values are used and the event is logged.

What else changed:

  • The flags in lib/galaxy/config/__init__.py were refactored to be sorted alphabetically.

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. Edit any of the aforementioned flags in the galaxy.yml.sample file and assess the behaviour on the client.
    2. Note: When setting a location, consult the list of supported locations mentioned in the docs.

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

Renni771 added 4 commits June 12, 2023 17:21
- Server location can be set as a valid ISO 3166 code. This is used to
  determine the carbon intensity based off of the galaxy instance server
  location
- A non-default power usage effectiveness value can be set
- Carbon emissions reporting can be toggled
- Add `geographical_server_location_code`, `geographical_server_location_name` and `carbon_intensity` flags.
- Alphabetically sort the list of config flags in galaxy init file.
- Add logic to set `geographical_server_location_name` and `carbon_intensity` based off of `geographical_server_location_code`.
@Renni771 Renni771 marked this pull request as ready for review July 1, 2023 08:41
@github-actions github-actions bot added this to the 23.2 milestone Jul 1, 2023
Copy link
Member

@bgruening bgruening left a comment

Choose a reason for hiding this comment

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

Really nice @Renni771 thanks a lot!

Anyone from the backend team that wants to have a look?

Copy link
Contributor

@davelopez davelopez left a comment

Choose a reason for hiding this comment

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

Really cool indeed!

Just a couple of comments below :)

lib/galaxy/managers/configuration.py Outdated Show resolved Hide resolved
lib/galaxy/config/__init__.py Show resolved Hide resolved
…ions flags

- `carbon_intensity` and `geographical_server_location_name` are
  automatically computed on startup and are never set by an admin. So
  they don't need to be in the config at all.
@Renni771
Copy link
Contributor Author

Renni771 commented Jul 6, 2023

Thanks for the comments, @davelopez! I've addressed points you've mentioned :)

@Renni771 Renni771 requested a review from davelopez July 6, 2023 11:45
Copy link
Contributor

@davelopez davelopez left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you @Renni771!

@ElectronicBlueberry ElectronicBlueberry merged commit dbc2b9b into galaxyproject:dev Jul 7, 2023
@ElectronicBlueberry
Copy link
Member

Thanks a lot @Renni771! Awesome work

@github-actions
Copy link

github-actions bot commented Jul 7, 2023

This PR was merged without a "kind/" label, please correct.

@@ -34,6 +34,7 @@

import yaml

from galaxy.carbon_emissions import get_carbon_intensity_entry
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 breaking the config module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this still causing an issue and, if so, how exactly is it breaking the config module?

Copy link
Member

Choose a reason for hiding this comment

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

it is, the config package can't import from carbon_emissions.

Copy link
Member

Choose a reason for hiding this comment

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

Work on a fix is underway in #17159

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the update!

@Renni771 Renni771 deleted the add-carbon-emissions-admin-configuration-options branch January 11, 2024 13:13
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.

6 participants