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: make iteration loops for simplified train consistent #263

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

olelod
Copy link
Contributor

@olelod olelod commented Oct 30, 2023

Why is this pull request needed?

def calculate_maximum_rate_given_outlet_pressure_single_calculation_point(
and
def calculate_enthalpy_change_head_iteration(
both have the same iterative loop changing z and kappa, but the algorithm for finding the max rate has an extra update in the end (
maximum_actual_volume_rate = maximum_rate_function(
). Removing this extra update solves the numerical problems reported in this issue.

Issues related to this change:

ECALC 381

@@ -194,7 +194,7 @@ def test_calculate_maximum_rate_given_outlet_pressure_all_calculation_points(
pressure_ratios = [1, 2, 3, 4, 5, 10, 100, 1000]

# These expected max rates are here just to assure stability in the results. They are not assured to be correct!
approx_expected_max_rates = [1116990, 1359255, 1536130, 1052085, 1052085, 1052085, 1052085, 1052085]
approx_expected_max_rates = [1116990, 1359022, 1536130, 1052085, 1052085, 1052085, 1052085, 1052085]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The iteration loop stops when the change in actual rate less than 0.1% from one iteration to the next. This PR basically removes the next update here. In the time step that changed here, the change was 0.09% when the iteration stops - the change in the next step (which is now removed for consistency) is 0.017%. For all other time steps the change in actual rate when the iterations stops are much less than 0.1%, and no visible difference in the test.

@olelod olelod marked this pull request as ready for review October 31, 2023 08:33
@olelod olelod requested a review from a team as a code owner October 31, 2023 08:33
@olelod olelod force-pushed the ECALC-381-handling-calculate-max-rate-rounding-error branch from 8b5161d to c32e9cc Compare October 31, 2023 08:33
@olelod olelod self-assigned this Oct 31, 2023
@olelod olelod force-pushed the ECALC-381-handling-calculate-max-rate-rounding-error branch from c32e9cc to 831120a Compare October 31, 2023 11:42
@olelod olelod merged commit b066c74 into main Oct 31, 2023
6 checks passed
@olelod olelod deleted the ECALC-381-handling-calculate-max-rate-rounding-error branch October 31, 2023 11:47
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