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: error when only venting emitters are specified for an installation #449

Merged
merged 2 commits into from
May 2, 2024

Conversation

frodehk
Copy link
Contributor

@frodehk frodehk commented Apr 24, 2024

ECALC-1061

Why is this pull request needed?

Error when venting emitters are defined alone, without a generator set - for a given installation. The reason is that the installation results are empty, and the installation regularity cannot be found for the particular venting emitters.

What does this pull request change?

More robust reading of regularity for venting emitters, independent of installation results: Reading from the asset object directly.

Issues related to this change:

https://equinor-ecalc.atlassian.net/browse/ECALC-1061?atlOrigin=eyJpIjoiNWUwMDMwMjkxMzc2NGEyMmJmMGY5ZTg3NTBlOThkZDQiLCJwIjoiaiJ9

@frodehk frodehk self-assigned this Apr 24, 2024
@frodehk frodehk requested a review from a team as a code owner April 24, 2024 12:11
Copy link
Contributor

@jsolaas jsolaas left a comment

Choose a reason for hiding this comment

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

Can we also add a test to make sure this doesn't happen again?

src/libecalc/application/graph_result.py Outdated Show resolved Hide resolved
@frodehk
Copy link
Contributor Author

frodehk commented Apr 30, 2024

Can we also add a test to make sure this doesn't happen again?

Have added this check:

assert isinstance(get_consumption_asset_result(model=dto_case.ecalc_model, variables=variables), EcalcModelResult)

@frodehk frodehk merged commit 017560d into main May 2, 2024
6 checks passed
@frodehk frodehk deleted the ECALC-1061-fix-bug-when-only-venting-emitters branch May 2, 2024 07:28
frodehk added a commit that referenced this pull request May 2, 2024
…on (#449)

* fix: error when only venting emitters due to installation regularity missing

ECALC-1061

(cherry picked from commit 017560d)
frodehk added a commit that referenced this pull request May 2, 2024
* fix: error when only venting emitters are specified for an installation (#449)

* fix: error when only venting emitters due to installation regularity missing

ECALC-1061

(cherry picked from commit 017560d)

* chore: update yaml oil type venting emitter (#448)

ECALC-1060

(cherry picked from commit 7997813)
equinor-schen pushed a commit that referenced this pull request Aug 23, 2024
…on (#449)

* fix: error when only venting emitters due to installation regularity missing

ECALC-1061
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