-
Notifications
You must be signed in to change notification settings - Fork 8
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
Duplication/Configuration Issues Related to ICEES+ Asthma & ICEES DILI, Development & Production Instances #207
Comments
Regarding my proposal, and looking ahead to new use cases, a merger of all use cases into one ICEES dev instance and one ICEES prod instance will only work if we are very careful about distinguishing feature variable names. For example, "Race" can be reused for all UNC Health data, but not for DILI Network data or EPR data. But yet, some variables are intended for reuse across use cases. For example, the exposures variables. Related, and as we've discussed, we also will need to rethink the use of SQLite as the ICEES db, given the limitations on column headers (2000). Hao's Dhall proposal might be the right solution, in terms of scalability, but I'm not qualified to make that decision. |
Related is issue #182. |
Update: 16339 - ICEES asthma dev, 16340 - copy ICEES asthma dev, 16341 - ICEES DILI dev, 16342 - ICEES asthma prod, 16343 - ICEES DILI prod
https://files.slack.com/files-pri/T0450N0TQ-F02NM1XBF52/image.png https://files.slack.com/files-pri/T0450N0TQ-F02NM1XBF52/image.png |
Next steps:
|
@hyi @kennethmorton @maximusunc : I ran a few KG one-hop tests using the three CURIES that only ICEES DILI should respond to. I also viewed Max's test results, using both QGraph and the ARAX viewer. I'm pretty sure that the KG one-hop endpoint is running pairwise correlations based on biolink:XXX categories/types, not the underlying data or input CURIE(s). I think that Hao's 'master plan' was to create a set of three YAML config files and then run code to break them into Dhall files, with me selecting variables for each ICEES instance, like this one. What he may have overlooked in the interim is a need for a fourth YAML config file to map between biolink:XXX and csv column headers. Regardless of the above, I think that fixes (1-5) above should resolve the issue in the near term, with an implementation of our planned redesign needed for the longer term. |
Issue resolved by splitting master ground truth all_features and identifiers YAML files into separate sets of files for each use case: asthma, dili, pcd, and covid. Completed for asthma, dili, and pcd. |
ICEES asthma prod and ICEES DILI prod deployed and tested. |
Resolved. |
This issue is focused primarily on issues related to the ICEES+ Asthma and ICEES+ DILI development and production instances, with the December 2021 Translator Demo in mind.
The results of various tests can be found here.
A summary of testing efforts follows below (from a Slack exchange between Kara, Kenny, and Max).
Okay, I just ran what I think is the final definitive test.
I looked up identifiers for DILIDx and AsthmaDx and also SexDILI and Sex2, using all four ICEES instances. DILIDx and SexDILI should only return responses from ICEES DILI; AsthmaDx and Sex2 should only return responses from ICEES Asthma.
Instead, all four ICEES instances returned responses to all four queries. The ICEES DILI and ICEES Asthma production instances returned incorrect identifiers, presumably because they are pointing to the outdated config files. The ICEES DILI and ICEES Asthma development instances returned correct identifiers, presumably because they are pointing to the ground truth config files.
Kenny: I bet if you ran the same asthma and DILI tests but pointed the asthma test at the DILI prod instance and the DILI test at the asthma prod instance, you'd receive the same results.
The actual data that are being returned to each query of dev and prod are correct. As noted above, but perhaps more firmly here, I'm proposing that we merge the asthma & DILI, dev & prod instances and expose one ICEES dev instance and one ICEES prod instance, using the ground truth config files.
The text was updated successfully, but these errors were encountered: