Skip to content

Commit

Permalink
Merge pull request #424 from mild-blue/speedup-fe
Browse files Browse the repository at this point in the history
Lower the number of matchings we store in db as it is not needed and slows down FE unnecessarily.
  • Loading branch information
kubantjan authored Feb 8, 2021
2 parents 2047755 + ea042dc commit 98d2359
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 25 deletions.
3 changes: 2 additions & 1 deletion tests/database/services/test_file_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ def test_saving_patients_from_obfuscated_excel(self):
max_cycle_length=100,
max_sequence_length=100,
max_number_of_distinct_countries_in_round=100,
use_split_resolution=False
use_split_resolution=False,
max_matchings_to_store_in_db=1000
),
txm_event).calculated_matchings_list)
self.assertEqual(358, len(all_matchings))
Expand Down
3 changes: 2 additions & 1 deletion tests/solvers/test_solve_from_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ def test_solve_from_configuration_forbidden_countries(self):
def test_solve_from_example_dataset(self):
txm_event_db_id = self.fill_db_with_patients(get_absolute_path(PATIENT_DATA_OBFUSCATED))
txm_event = get_txm_event(txm_event_db_id)
configuration = Configuration(use_split_resolution=True)
configuration = Configuration(use_split_resolution=True,
max_matchings_to_store_in_db=1000)
solutions = list(solve_from_configuration(configuration, txm_event).calculated_matchings_list)
self.assertEqual(947, len(solutions))

Expand Down
3 changes: 0 additions & 3 deletions tests/web/test_swagger_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ def test_server_logged_in(self):
'post': {
f'{API_VERSION[1:]}/{USER_NAMESPACE}/login': [401],
f'{API_VERSION[1:]}/{USER_NAMESPACE}/otp': [403],
# TODO remove this in https://github.com/mild-blue/txmatching/issues/372
f'{API_VERSION[1:]}/{TXM_EVENT_NAMESPACE}/{{txm_event_id}}/'
f'{MATCHING_NAMESPACE}/calculate-for-config': [400]
},
'put': {
f'{API_VERSION[1:]}/{USER_NAMESPACE}/otp': [403],
Expand Down
4 changes: 4 additions & 0 deletions txmatching/configuration/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class Configuration:
forbidden_country_combinations: Pairs of countries which do not support mutual transplantations.
manual_donor_recipient_scores: Manual setting of score for tuple of recipient and donor.
max_matchings_to_show_to_viewer: Viewer cannot see all the details of the app.
max_matchings_to_show_to_store_in_db: Max matchings we keep in database and show to EDITOR.
max_allowed_number_of_matchings: Max allowed number of matchings the user is allowed to work with.
"""
scorer_constructor_name: str = 'HLAAdditiveScorer'
solver_constructor_name: str = 'AllSolutionsSolver'
Expand All @@ -63,6 +65,8 @@ class Configuration:
# TODO: https://github.com/mild-blue/txmatching/issues/373 change field type to set
manual_donor_recipient_scores: List[ManualDonorRecipientScore] = field(default_factory=list)
max_matchings_to_show_to_viewer: int = field(default=10, compare=False)
max_matchings_to_store_in_db: int = field(default=100, compare=False)
max_allowed_number_of_matchings: int = field(default=10000, compare=False)

def __eq__(self, other):
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@
'max_matchings_to_show_to_viewer': fields.Integer(
required=False,
example=_default_configuration.max_matchings_to_show_to_viewer
),
'max_matchings_to_store_in_db': fields.Integer(
required=False,
example=_default_configuration.max_matchings_to_store_in_db
),
'max_allowed_number_of_matchings': fields.Integer(
required=False,
example=_default_configuration.max_allowed_number_of_matchings
)
}
)
22 changes: 12 additions & 10 deletions txmatching/solve_service/solve_from_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
from txmatching.solvers.solver_from_config import solver_from_configuration

logger = logging.getLogger(__name__)
MAX_ALLOWED_NUMBER_OF_MATCHINGS = 1000000
MAX_NUMBER_OF_MATCHINGS_TO_STORE = 1000


def solve_from_configuration(configuration: Configuration, txm_event: TxmEvent) -> PairingResult:
Expand All @@ -31,8 +29,10 @@ def _solve_from_configuration_unsafe(configuration: Configuration, txm_event: Tx
all_matchings = solver.solve()
matching_filter = filter_from_config(configuration)

matchings_filtered_sorted, all_results_found, matching_count = _filter_and_sort_matchings(all_matchings,
matching_filter)
matchings_filtered_sorted, all_results_found, matching_count = _filter_and_sort_matchings(
all_matchings,
matching_filter,
configuration)

logger.info(f'{matching_count} matchings were found.')

Expand All @@ -45,7 +45,8 @@ def _solve_from_configuration_unsafe(configuration: Configuration, txm_event: Tx


def _filter_and_sort_matchings(all_matchings: Iterator[MatchingWithScore],
matching_filter: FilterBase
matching_filter: FilterBase,
configuration: Configuration
) -> Tuple[List[MatchingWithScore], bool, int]:
matchings_heap = []
all_results_found = True
Expand All @@ -61,16 +62,17 @@ def _filter_and_sort_matchings(all_matchings: Iterator[MatchingWithScore],
)

heapq.heappush(matchings_heap, matching_entry)
if len(matchings_heap) > MAX_NUMBER_OF_MATCHINGS_TO_STORE:
if len(matchings_heap) > configuration.max_matchings_to_store_in_db:
heapq.heappop(matchings_heap)

if i % 100000 == 0:
logger.info(f'Processed {i} matchings')

if i == MAX_ALLOWED_NUMBER_OF_MATCHINGS - 1:
logger.error(f'Max number of matchings {MAX_ALLOWED_NUMBER_OF_MATCHINGS} was reached. Returning only '
f'best {MAX_NUMBER_OF_MATCHINGS_TO_STORE} matchings from {MAX_ALLOWED_NUMBER_OF_MATCHINGS}'
f' found up to now.')
if i == configuration.max_allowed_number_of_matchings - 1:
logger.error(
f'Max number of matchings {configuration.max_allowed_number_of_matchings} was reached. '
f'Returning only best {configuration.max_matchings_to_store_in_db} matchings from '
f'{configuration.max_allowed_number_of_matchings} found up to now.')
all_results_found = False
break

Expand Down
23 changes: 13 additions & 10 deletions txmatching/web/api/matching_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

from txmatching.auth.auth_check import require_valid_txm_event_id
from txmatching.auth.data_types import UserRole
from txmatching.auth.exceptions import CachingNotReadyException
from txmatching.auth.request_context import get_user_role
from txmatching.auth.user.user_auth_check import require_user_login
from txmatching.data_transfer_objects.configuration.configuration_swagger import \
Expand All @@ -17,11 +16,15 @@
CalculatedMatchingsJson
from txmatching.data_transfer_objects.txm_event.txm_event_swagger import \
FailJson
from txmatching.database.services import solver_service
from txmatching.database.services.config_service import (
configuration_from_dict, find_configuration_db_id_for_configuration)
from txmatching.database.services.matching_service import (
create_calculated_matchings_dto, get_matchings_detailed_for_configuration)
from txmatching.database.services.txm_event_service import get_txm_event
from txmatching.solve_service.solve_from_configuration import \
solve_from_configuration
from txmatching.utils.logged_user import get_current_user_id
from txmatching.web.api.namespaces import matching_api

logger = logging.getLogger(__name__)
Expand All @@ -45,17 +48,17 @@ class CalculateFromConfig(Resource):
def post(self, txm_event_id: int) -> str:
txm_event = get_txm_event(txm_event_id)
configuration = configuration_from_dict(request.json)
user_id = get_current_user_id()
maybe_configuration_db_id = find_configuration_db_id_for_configuration(txm_event=txm_event,
configuration=configuration)
if maybe_configuration_db_id:
matchings_detailed = get_matchings_detailed_for_configuration(txm_event, maybe_configuration_db_id)
else:
raise CachingNotReadyException('Caching is not ready')
# TODO move this commented out section to another endpoint in and also remove the raising of error and
# return some object telling FE that the cached mathcing does not exist.
# https://github.com/mild-blue/txmatching/issues/372
# pairing_result = solve_from_configuration(configuration, txm_event=txm_event)
# solver_service.save_pairing_result(pairing_result, user_id)
if not maybe_configuration_db_id:
pairing_result = solve_from_configuration(configuration, txm_event=txm_event)
solver_service.save_pairing_result(pairing_result, user_id)
maybe_configuration_db_id = find_configuration_db_id_for_configuration(txm_event=txm_event,
configuration=configuration)

assert maybe_configuration_db_id is not None
matchings_detailed = get_matchings_detailed_for_configuration(txm_event, maybe_configuration_db_id)

calculated_matchings_dto = create_calculated_matchings_dto(matchings_detailed, matchings_detailed.matchings)

Expand Down
6 changes: 6 additions & 0 deletions txmatching/web/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,18 @@ definitions:
items:
$ref: '#/definitions/ManualRecipientDonorScore'
type: array
max_allowed_number_of_matchings:
example: 10000
type: integer
max_cycle_length:
example: 4
type: integer
max_matchings_to_show_to_viewer:
example: 10
type: integer
max_matchings_to_store_in_db:
example: 100
type: integer
max_number_of_distinct_countries_in_round:
example: 3
type: integer
Expand Down

0 comments on commit 98d2359

Please sign in to comment.