-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: add compressor inlet- and outlet pressures to models/train level #152
Conversation
src/ecalc/cli/tests/snapshots/test_app/test_json_true/test_v3.json
Outdated
Show resolved
Hide resolved
src/ecalc/cli/tests/snapshots/test_app/test_json_true_detailed_output/test_full_json_v3.json
Outdated
Show resolved
Hide resolved
We should have consumer systems that return sth else than |
There is also a mismatch with the train/model timevector and the individual variables (e.g. rates, pressure etc) per now, i.e. in the way it is implemented in this PR. Think there is a need to slice the variables map as input to the temporal expression, to match correct time intervals. |
Seems like the csv-tests are the only ones using the advanced yaml (with more advanced settings for compressors and compressor systems) - but data for models/trains are only dumped to json. I think we should extend the json-tests with one using the advanced yaml-file. |
yes, or add a relevant test (can be really simple) to the existing json-export-tests |
src/ecalc/libraries/libecalc/common/libecalc/core/graph_result.py
Outdated
Show resolved
Hide resolved
96e310b
to
aada5bb
Compare
src/ecalc/libraries/libecalc/common/libecalc/common/utils/rates.py
Outdated
Show resolved
Hide resolved
0f36db6
to
c60d50b
Compare
src/ecalc/libraries/libecalc/common/libecalc/core/graph_result.py
Outdated
Show resolved
Hide resolved
src/ecalc/libraries/libecalc/common/libecalc/core/graph_result.py
Outdated
Show resolved
Hide resolved
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.
all checks are ok, not sure what you meant by erroneous snapshot? did you perhaps just change to the values we got in CICD , but that it is still failing localyl for you?
All tests are ok now, as the snapshot assert is removed, i.e. not using snapshot. Originally it was using snapshot. The snapshot is generated by copying the output json from the test, to the snapshot directory. Then running the test with --snapshot-update. Then, locally all is fine but it fails in the pipeline (in the 10th decimal I think). But as you see, snapshot is not used now. |
fe15926
to
9735059
Compare
Why is this pull request needed?
Add compressor inlet- and outlet pressures from input data to models/train level (as requested inlet- and outlet pressures). It is initially implemented at component level, this is now removed - as it is most relevant for the users at train-level. When implemented in web, the input pressures can be plotted together with calculated pressures. Make it easier to understand that in case of pressure control mechanisms are active, the requested (input) inlet and outlet pressures may differ from what is actually calculated by eCalc.
What does this pull request change?
Issues related to this change:
https://equinor-ecalc.atlassian.net/browse/ECALC-121
https://equinor-ecalc.atlassian.net/browse/ECALC-92