monarch-initiative / mondo

Mondo Disease Ontology
http://obofoundry.org/ontology/mondo
Creative Commons Attribution 4.0 International
223 stars 51 forks source link

Add ncbigene pipeline with new source #7857

Closed matentzn closed 12 hours ago

matentzn commented 2 days ago
sabrinatoro commented 2 days ago

Note: I do not have enough memory to run the import refresh. I tried with 16, 20, 25, and 30 GB

twhetzel commented 2 days ago

@sabrinatoro Are these the steps you are following: https://mondo.readthedocs.io/en/stable/editors-guide/import-terms-for-logical-axioms/#import-terms-into-mondo-for-use-in-logical-axioms

Let me know if you want me to try since I have more memory available on this laptop.

matentzn commented 2 days ago

@sabrinatoro I agree with @twhetzel thats probably for the best for now. I think I can reduce the memory foot print, I have reached out for some help.

In the meantime, I have updated the import so this PR is ready to go.

matentzn commented 1 day ago

@sabrinatoro If you like you can try it again, I reduce the memory footprint a bit and it should work withMEMORY_GB=24. But you can also just merge it, let Trish do import refresh in the meantime via SOP and punt making this less memory intensive moving forward.

sabrinatoro commented 1 day ago

@twhetzel could you please try running the import, and confirm that it works (ie that we have the non-human genes in Mondo; the list is in the manual_seed.txt which was updated in the PR). If it works, then merge this PR.

twhetzel commented 1 day ago

@matentzn I refreshed the imports following this Mondo Docs using this ncbigene-import-full feature branch.

This is the summary of changes: src/ontology/imports/go_terms.txt | 2 + src/ontology/imports/hp_terms.txt | 1 + src/ontology/imports/merged_import.owl | 308 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- src/ontology/imports/ncbitaxon_terms.txt | 14 +++++

I wasn't expecting additional changes since you ran and committed these files. Is this^^ expected?

PS - let me know if you want me to push these changed files here to review. PPS - I did open mondo-edit.obo in Protege and confirmed I could find http://identifiers.org/ncbigene/481341 and http://identifiers.org/ncbigene/397506

matentzn commented 16 hours ago

@twhetzel If the 308 changes in merged imports look mostly harmless, its ok! I would like to see it, you commit your changes here, no issue.

twhetzel commented 15 hours ago

@matentzn sure, I just pushed those changed files to this PR.

matentzn commented 14 hours ago

@twhetzel thanmk you, the changes are all expected! Awesome. Ready to merge!