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

Fix uncontrolled charging RAMP profile #358

Merged

Conversation

FraSanvit
Copy link
Contributor

@FraSanvit FraSanvit commented Apr 12, 2024

Fixes #287.

Checklist

Any checks which are not relevant to the PR can be pre-checked by the PR creator. All others should be checked by the reviewer. You can add extra checklist items here if required by the PR.

  • CHANGELOG updated
  • Minimal workflow tests pass
  • Tests added to cover contribution
  • Documentation updated
  • Configuration schema updated

Copy link
Member

@brynpickering brynpickering left a comment

Choose a reason for hiding this comment

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

Nice and small, very good. If we have improved input data from RAMP now (e.g. more countries), it would be good to have new demand, plugin, and battery consumption profiles all in the same zenodo record. Do you have the other two profiles from the run you did?

config/schema.yaml Outdated Show resolved Hide resolved
scripts/transport/road_transport_timeseries.py Outdated Show resolved Hide resolved
@FraSanvit
Copy link
Contributor Author

FraSanvit commented Apr 12, 2024

Nice and small, very good. If we have improved input data from RAMP now (e.g. more countries), it would be good to have new demand, plugin, and battery consumption profiles all in the same zenodo record. Do you have the other two profiles from the run you did?

Not really. The plug-in profiles are custom outputs, but I can easily regenerate all the three timeseries..

@@ -107,6 +107,10 @@ properties:
type: string
pattern: ^(https?|http?):\/\/.+
description: Web address of electric vehicle data.
uncontrolled-ev-data:
Copy link
Member

Choose a reason for hiding this comment

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

data -> profiles?

config/default.yaml Outdated Show resolved Hide resolved
Copy link
Member

@brynpickering brynpickering left a comment

Choose a reason for hiding this comment

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

Great, just one last thing and then you can merge this in: update the changelog!

@brynpickering
Copy link
Member

@FraSanvit if you could re-generate all the profiles we need for euro-calliope with the latest RAMP (incl. the new countries) that would be great. Not for this PR, but at some point.

@adrienmellot
Copy link
Contributor

Great, just one last thing and then you can merge this in: update the changelog!

I've looked at the changelog and I am not sure we can really update it with this PR because we're fixing something that is 'added' to v1.2.0. What do you think?

@timtroendle
Copy link
Member

You could simply add this PR number to that existing entry.

@timtroendle
Copy link
Member

I've updated CHANGELOG accordingly.

@brynpickering brynpickering merged commit cf38660 into calliope-project:develop May 17, 2024
4 checks passed
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.

Ensure existing RAMP profiles are appropriate for full electrification
4 participants