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 NPI triggers to GLEAM modelling #536

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

gavento
Copy link
Contributor

@gavento gavento commented Sep 17, 2020

Tracking issue: #531

  • Created template sheet to be downloaded as CSV
    • The table only contains setting fixed beta, ignoring traces - this is NOT the complete feature. It would be best to use multipliers but that requires scenario generation integration.
  • Added command UpdateNPITriggers to add NPI trigers to a (computed) batch file, creating a batch file with updated definitions (with NPIs added) and truncated timeseries (to contain only valid data).
  • Updated WebExport to handle NaNs (although Plotly displays them as 0 .. we would need to remove them in JS, I think)
  • See comment for example commands
  • The command and the update loop work, but the result is buggy.
  • I did not manage to hook this machinery into scenario generation, so the exceptions are added on top of existing ones.
    • This seems to be a large source of problems, I am not sure how to proceed.
    • Updating existing exceptions (in the XML) is almost undoable, as they combine different regions (countries, basins, ..) and dates. It is not clear how GLEAM prioritizes them if we add new ones to overwrite old ones.

@lgtm-com
Copy link

lgtm-com bot commented Sep 17, 2020

This pull request introduces 2 alerts when merging 04c8b77 into 57bb0e7 - view on LGTM.com

new alerts:

  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Sep 18, 2020

This pull request introduces 2 alerts when merging 786f59d into 57bb0e7 - view on LGTM.com

new alerts:

  • 2 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Sep 18, 2020

This pull request introduces 2 alerts when merging 566dca8 into 57bb0e7 - view on LGTM.com

new alerts:

  • 2 for Unused import

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.

2 participants