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 risk to dd #32

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

Add risk to dd #32

wants to merge 28 commits into from

Conversation

adrianaghiozzi
Copy link
Contributor

No description provided.

@adrianaghiozzi adrianaghiozzi requested a review from orso82 October 23, 2024 20:27
"type": "static",
"units": "$/kWh"
},
"costing.model": {
Copy link
Member

Choose a reason for hiding this comment

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

@adrianaghiozzi if you want to add the model name, we should try to conform to what IMAS generally does for all of its IDSs. I'll add some logic to generate_dd to copy the .code tree under the IDSs that we have added. This should allow you to use costing.code.name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@orso82 Thanks, that would be great - this is just needed for the risk actor because the costs are grouped differently depending on whether it's ARIES or Sheffield (costing each coil system separately vs. all together) so the model determines how many fields are needed in dd.risk.engineering.loss

Copy link
Member

Choose a reason for hiding this comment

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

Uhm... I have not taken a look at the code, but it sounds like you are hardcoding the number of fields based on the model, which is definitely not a robust thing to do. Perhaps it would be best to just pick a model (say ARIES), and use the TF costs from that model for the evaluation of risk.

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