geneontology / amigo

AmiGO is the public interface for the Gene Ontology.
http://amigo.geneontology.org
BSD 3-Clause "New" or "Revised" License
29 stars 17 forks source link

Change default closure to isa_partof from regulates #620

Closed cmungall closed 2 years ago

cmungall commented 3 years ago

Ideally this would be one parameter but I think there are about 6 or 7 places in the .js to change: https://github.com/geneontology/amigo/search?q=regulates_closure

This would make AmiGO consistent with Panther.

Estimated time: small

Note: I think we still need to do some investigation as to the implications of including/excluding BP-regulates-BP, but the immediate priority is consistency

kltm commented 3 years ago

Probably not? IIRC, it's certainly been changed before without too much mechanical fuss. There are few things that I think would be good to consider/get ready for:

1) track down the persons/reason for the initial change to regulates--I think that was probably done some time ago and outside the tracker. Related tickets may be:
https://github.com/geneontology/amigo/issues/210
https://github.com/geneontology/amigo/issues/602 2) What else might we want to change; e.g. what does the ribbon do? 2) how to communicate this change

kltm commented 3 years ago

Poking around in the code a little just to get an idea, it would seem that it is mostly wired on the surface on a client-by-client basis. That means there would be a bunch of changes, but I suspect mostly replacement. A little more work would need to be done to get it "centralized" in the config. A little more irritating, there are also fields in GOlr, like regulates_transitivity_graph_json, which seem to have been created after the switch from isa_part_of in the first place that have no isa_part_of analog. The schema would have to be extended and those would need to be added to the loader. These seem to be a bit more embedded where they're used in clients and might require a little fiddling to clean up.

cmungall commented 3 years ago
  1. it started out this way, going back to when all the regulates relations were part-ofs. And yes this solves those two issues (those issues are about blocking propagation solely in MF)
  2. good q. ribbon uses http://api.geneontology.org/api

image

so the default gp2term relationship is involved-in, i.e. use the isa-partof-closure

  1. good point. Shall we make the PR and then bring this up next managers call?
pgaudet commented 3 years ago

Can this be configurable by the user with some checkbox? Depending on what you are looking at, it may be interesting to get a wider or narrow range of terms.

pgaudet commented 3 years ago

Added to next week's managers call.

cmungall commented 3 years ago

We have other tickets for making it easy to switch between the views (will link later, sorry). I suggest scoping this ticket to just changing the default behavior.

kltm commented 3 years ago

While there will be a bunch of changes to AmiGO code, that's mostly "surface" (if possibly fiddly). Most of the work and time on this will actually be pipeline and deployment, so I'd be fine treating it as a pipeline maintenance/bugfix (for data), rather than an AmiGO issue per se.

ukemi commented 3 years ago

We should also keep in mind that the GPAD files generated from Noctua models will create an 'acts_upstream_of' X annotation for any annotation to a regulation of X term. Although closure is a good thing to address, the issue is more complex (isn't it always?).

kltm commented 3 years ago

Setup work spaces: https://github.com/geneontology/amigo/tree/issue-620-change-default-closure https://github.com/geneontology/pipeline/tree/issue-amigo-620-change-default-closure https://build.geneontology.org/job/geneontology/job/pipeline/job/issue-amigo-620-change-default-closure/

kltm commented 3 years ago

Should now have GOlr Solr index building image with geneontology/golr-autoindex:f815e3e9004f96493f29a80a73526c91c2a4c25d_2021-10-01T162208_issue-620-change-default-closure Will test after release.

kltm commented 3 years ago

@cmungall Noting here that "generate_val_statistics.js" uses regulates_closure_label. Leaving this as is for now.

kltm commented 3 years ago

@cmungall As a temporary testing measure, I've put a small load (essentially a few minor GAFs and MGI, see: http://amigo-exp.geneontology.io/amigo/load_details) onto http://amigo-exp.geneontology.io/amigo . This will be reverted in case we need to fail over or when initial testing is done and we are ready to move on to staging (TODO). If you'd like, now that we've done initial build debugging and decrufting, we can run the full load and put that up.

Poking around a little, a few notes:

kltm commented 2 years ago

After discussion with @cmungall , we'll keep the topology load as-is for the moment. Now rebuilding with full load for more testing.

kltm commented 2 years ago

@cmungall Full load (delta recent IEA issues) now available on amigo-exp.

kltm commented 2 years ago

Notes from conversation with @cmungall : 1) limit state toggle to the term page (only);
may be possible to implement by gaming the bookmarks
fine to change the URL parameter to do this to something more general 2) add filters to annotation search

cmungall commented 2 years ago

https://docs.google.com/presentation/d/15NOq1x27t43K6TDMoPnnVQzWFbcgRfix96ng4j8YNNs/edit#slide=id.g107a668700d_0_8

kltm commented 2 years ago

This (contents and UI) has passed review and can start filtering to production.

cmungall commented 2 years ago

It would be good to document what we have done here

These slides. are from before the last GO meeting: https://docs.google.com/presentation/d/19UoPyCUBIOW5hrLG27ARJMNsjyOJ_c0Qv-Iz2x-CS14/edit#slide=id.p

these could be cloned and updated with new figures

kltm commented 2 years ago

When complete, reminder for @kltm to update link in doc: https://docs.google.com/document/d/1906vUn0DD160zofo_CLT414_X2uqh0aoA7XmHKyIaR8/edit#heading=h.jglmdvcq6xxp Noting this it is unlikely to be this release cycle (a lot of ongoing problems), so probably the next. Otherwise, the experimental link can be more-or-less stably used as this is will be the new standard.

kltm commented 2 years ago

Remaining TODO:

kltm commented 2 years ago

In production. Anything further is a bug.