Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
refactor: upgrade CA-NS with event classes #6050
refactor: upgrade CA-NS with event classes #6050
Changes from 4 commits
459e1cf
0de7079
9be429a
6eb5409
5f96eeb
3be20e5
b57a69d
dc2b5ce
4b60fc8
c5162a7
792e490
a1de3cb
96a5b6c
65a4769
8ba6905
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly related to this refactor, but just to mention. The assumption that all imports are from NB is now problematic because Nova Scotia's interconnection to Newfoundland is now live. But I've never seen live data on its operation, the closest thing was that it amounted to 14% of NS supply in 2023H1, per https://www.nspower.ca/cleanandgreen/clean-energy "Our Energy Stats" chart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reference related to your comment: the daily generation reports for Newfoundland might be usable as a source for the interconnector data, see #2541 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, look: I found a page on the NSPower website that has the information you need (Maritime Link interconnector) and more, updated roughly every 5 minutes, with more granular data than the current source: it's called Daily Report, but under the heading "Current System Conditions" you'll find "10 minute averages" of e.g. load, wind, and imports and exports through various interconnectors that are at most five minutes old.
Edit: well, turns out you found it at some point as well: #3206 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I dug deeper into this, and it turns out we aren't actually using this exchange data since we get the data on the NS-NB interconnection from New Brunswick Power, not from NSPower. So it might be best to just delete the exchanges-related part, or, alternatively, switch to obtaining exchange info from the source in #6317 (and perhaps implement the NS-Newfoundland interconnector as well). But that could be done in a separate issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @VIKTORVAV99, any thoughts on this? Should we simply delete the exchange parser if it's not used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, my idea was to delete the exchange parser now so we are more certain we don't accidentally fetch incorrect data, and then re-add an exchange parser based on the new source from #6317 in a separate pull request.
But doing both in this PR is also acceptable, if you've got the time for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, gotcha! It's not entirely clear to me what needs to be done to implement the new exchange parser (there are a lot of links to follow and I'm a bit lost), so I'll just delete it here and we can address it separately. Would those changes have any impact on the production parser?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exchange stuff shouldn't. The "Current System Conditions" also has wind generation, but as the page has data updated roughly every 5 minutes I think it'll be hard to integrate with the hourly data provided by the current source – and probably not that useful either, if the current source stays online.
As for implementing the exchange parser, the short summary is: from https://www.nspower.ca/oasis/system-reports-messages/daily-report, under "Current System Conditions", grab:
The rest of the stuff in the issue is useful for exchange configuration and historical data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for clarifying! That doesn't sound as arduous as I thought; maybe I can roll it into this PR. I'll try to carve out some time to work on it this weekend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, that wasn't so bad! A few notes:
CA-NL-NF->CA-NS
exchange, but this is my first time doing so, so I very well could have done something wrong. In particular, I haven't tested how it will look because I don't have the front-end set up on my machine (I used Google Maps for the coordinates and a protractor for the rotation 😁). Further, I wasn't sure whether to useCA-NL-LB
orCA-NL-NF
for the other zone (I chose the latter based on the physical location of the link).CA-NB->CA-NS
exchange, but the back-end is still configured to use the NB parser for this pair.