[fix/feat] Remove duplicate functions in grid_utils#815
Open
maxbriel wants to merge 3 commits intoPOSYDON-code:v2.3from
Open
[fix/feat] Remove duplicate functions in grid_utils#815maxbriel wants to merge 3 commits intoPOSYDON-code:v2.3from
grid_utils#815maxbriel wants to merge 3 commits intoPOSYDON-code:v2.3from
Conversation
…ixes POSYDON-code#442) The three functions in posydon/utils/gridutils.py that duplicated logic already present in posydon/utils/common_functions.py have been refactored: - kepler3_a(P, m1, m2) -> delegates to orbital_separation_from_period - T_merger_a(a, m1, m2) -> delegates to inspiral_timescale_from_separation (eccentricity=0, converts Myr->Gyr) - T_merger_P(P, m1, m2) -> delegates to inspiral_timescale_from_orbital_period (eccentricity=0, converts Myr->Gyr) Original function signatures and return units are preserved for backward compatibility. beta_gw is kept as-is (no equivalent in common_functions). test_gridutils.py: elements set updated to include the three newly imported names (inspiral_timescale_from_orbital_period, inspiral_timescale_from_separation, orbital_separation_from_period). All 33 gridutils tests and 138 common_functions tests pass.
…nctions - Remove kepler3_a, T_merger_P, T_merger_a entirely from gridutils.py; their canonical counterparts in common_functions.py are now the only implementations. - Move beta_gw from gridutils.py into common_functions.py so all GW inspiral helpers live in one place. - Replace the internal T_merger_P call in convert_output_to_table with a direct call to inspiral_timescale_from_orbital_period. - Drop now-unused constant imports from gridutils.py (Msun, Rsun, clight, cgrav, secyear). - Update test_gridutils.py: remove elements and tests for the four deleted functions. - Update test_common_functions.py: add beta_gw to the elements set and add test_instance_beta_gw + test_beta_gw. All 25 gridutils and 140 common_functions tests pass.
maxbriel
changed the title
grid_utilsgrid_utils
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
We had some duplicate functions in
grid_utils.py, which were also present in ourcommon_functions.py.This PR removes the duplicate functions and replaces them with those in
common_functions.Related to issue #442, though I'm not sure if all parts of that issue are addressed with the PR (i.e. the flow chart bulletpoint).