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

Changing Creative mode threshold from results to time #808

Closed
tokebe opened this issue Apr 11, 2024 · 11 comments
Closed

Changing Creative mode threshold from results to time #808

tokebe opened this issue Apr 11, 2024 · 11 comments
Assignees
Labels
enhancement New feature or request

Comments

@tokebe
Copy link
Member

tokebe commented Apr 11, 2024

Currently, BTE will run templates until it reaches 500 results. If BTE takes 4 minutes to retrieve 499 results from template 1, it'll run template 2 and go over time. Simultaneously, if BTE takes 1 minute to retrieve 500 results from template 1, it won't use any remaining time to check if template 2 has better results.

We should instead check if more than x time is remaining (perhaps 2.5 minutes?), and run the next template if so. We could also include a dryrun step to check how many meta-edges are going to be hit by a given template as a heuristic for how long we might expect a template to take, and compare that against time remaining. This should allow BTE to get the best results given the time remaining, even if it sometimes returns <500 results due to time.

This will require further investigation and discussion before any sort of implementation work can be done.

@tokebe
Copy link
Member Author

tokebe commented Apr 11, 2024

Note that such an implementation may significantly help results/performance from #794

@colleenXu colleenXu added needs discussion enhancement New feature or request labels Apr 16, 2024
@tokebe
Copy link
Member Author

tokebe commented Apr 17, 2024

After further discussion, we've settled on logic to start testing by:

  • Before the templates, start a timer.
  • After any given template, if the time remaining (5 minutes - timer) is greater than the expected next template time + 30 seconds, continue on to the next template. Otherwise, wrap-up and finish query execution.

Implementation requires a few things:

  • A given template group should have an expectedTime property which can be checked against. For initial testing, let's assume 2 minutes (expressed in seconds). This will be faster to check against than a dry-run metaEdges heuristic, and we can get actual run-time averages for templates rather than best-guess.
  • New timer logic during the template loop
  • Isolated testing to determine expected template timings (this can be the last step or performed simultaneously by another dev)

@tokebe
Copy link
Member Author

tokebe commented Apr 17, 2024

@rjawesome I'm assigning this issue to you given your familiarity with the inferred mode handler at this point. As always let us know if you have any questions.

@colleenXu
Copy link
Collaborator

colleenXu commented Apr 19, 2024

I discussed the "testing" aspect with Jackson earlier today.

We both imagined an automated testing framework to run templates with a list of input IDs, and record run-time info. Other info could also be helpful like: how many MetaEdges, how many subqueries, how long scoring/the NGD step is taking...

For lists of input IDs, we could use:

@tokebe
Copy link
Member Author

tokebe commented Apr 22, 2024

Note: Much of https://github.com/biothings/bte-auto-demos could be re-purposed for this kind of testing (removing some of the unneeded automated server framework, re-running, caching, etc.)

@rjawesome
Copy link
Contributor

rjawesome commented Apr 27, 2024

Basic implementation with durationMin property on query template JSONs has been completed in creative-timer branch in query_handler. I'm also working on the automated testing thing in biothings_explorer/performance-test/template_test.js (creative-timer branch)

@rjawesome
Copy link
Contributor

rjawesome commented May 1, 2024

A working version of the automated testing framework has been completed in biothings_explorer/performance-test/template_test.js (requires server to be running on localhost or prod) & biothings_explorer/performance-test/template_test_threaded.js (this program starts its own threads to run queries & should be faster, server should not be running).

The creative mode queries that are used have to be placed in the biothings_explorer/performance-test/template_data folder. The script will automatically detect the appropriate creative mode templates for each query and time them.

After the script is finished, it will give an output looking like this (for all tempaltes that were ran from the creative mode queries supplied).

{
  'Chem-treats-DoP.json': { count: 1, totalMs: 18491, avgMin: 0.31 },
  'Chem-treats-PhenoOfDisease.json': { count: 1, totalMs: 600000, avgMin: 10 },
  'Chem-regulates,affects-Gene-biomarker,associated_condition-DoP.json': { count: 1, totalMs: 600000, avgMin: 10 }
}

(I hard coded it so a timeout [>5 minutes] is recorded as 10 minutes)

@tokebe
Copy link
Member Author

tokebe commented May 2, 2024

@rjawesome If you make a draft PR, it'll be a little easier to comment on code review. On line 532, you have
const queryTime = durationMin * 60 * 1000 ?? DEFAULT_QUERY_TIME;. If durationMin is not set for a template, you'll get undefined * 60 * 1000 which evaluates to NaN. NaN ?? value will evaluate to NaN rather than value, which will probably behave in an unintended manner.

Additionally, you've currently left in the creative results threshold, which should be removed (I imagine you're getting to that).

Otherwise, I like the implementation -- skipping a template when there isn't enough time for it rather than just stopping all template execution there is a good call, and means we could hypothetically run shorter-running but lower-priority templates under some circumstances.

@rjawesome
Copy link
Contributor

Seeing that a lot of the queries that will be used for the "testing" aspect have similar query structures w/ changing IDs, I have added an "id template" feature to the "testing" program (performance-test/template_test.js and performance-test/template_test_threaded.js).

For example, first.json has the following contents. It will run the query two times in testing, the first time replacing {ID} with MONDO:0002909 and the second time replacing {ID} with MONDO:0019499

{
    "message": {
        "query_graph": {
            "nodes": {
                "n0": {
                    "ids": "{ID}"
                },
                "n1": {
                    "categories": ["biolink:Drug"]
               }
            },
            "edges": {
                "e0": {
                    "subject": "n1",
                    "object": "n0",
                    "predicates": ["biolink:treats"],
                    "knowledge_type": "inferred"
                }
            }
        }
    },
    "ids": ["MONDO:0002909", "MONDO:0019499"]
}

@tokebe
Copy link
Member Author

tokebe commented May 7, 2024

I think I understand the approach here -- you're eventually going to have to make 2 more templates alongside first.json to handle the other two templateGroups, which each require a specific qualifier on the inferred edge.

@tokebe
Copy link
Member Author

tokebe commented Jul 10, 2024

Superseded by #824

@tokebe tokebe closed this as completed Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants