[Review Requested] Update Underway #986

cgreene · 2020-02-10T18:21:40Z

Modify the manuscript to note that an update is underway. I'll group a few more changes in here.

AppVeyorBot · 2020-02-10T18:26:20Z

AppVeyor build 1.0.16 for commit 29b7ea5 by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

AppVeyorBot · 2020-02-10T19:19:19Z

AppVeyor build 1.0.17 for commit 7facd07 by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

cgreene · 2020-02-10T19:23:25Z

@dhimmel and @agitter : I'm curious about what you think of breaking out authors separately like this while modifications are happening.

AppVeyorBot · 2020-02-10T19:25:50Z

AppVeyor build 1.0.18 for commit d90da3d by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

AppVeyorBot · 2020-02-10T19:30:35Z

AppVeyor build 1.0.19 for commit 8fb8b56 by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

…to most recent approval)

AppVeyorBot · 2020-02-10T19:55:50Z

AppVeyor build 1.0.20 for commit cd2bfc6 by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

agitter

This overall strategy works for me. It won't be the final format we settle on when Version 2.0 is finished, but I like the idea of indicating new people have contributed to this form of the manuscript.

I have a few specific comments or suggestions and approve the overall design.

content/00.front-matter.md

agitter · 2020-02-12T16:40:43Z

content/00.front-matter.md

+  {%- endif -%}
+{% endfor %}
+
+<sup>♠</sup> --- Author order for version 2.0 is currently arbitrary.<br>


Should we make it alphabetical so we can give clear instructions to new Version 2.0 authors? That may be too tricky if some authors are both v1 and v2 authors.

I wonder if there's a way to have the v1 field become the author contributions grouping there and the v2 field become the same (instead of just true/empty). That opens up the potential to put author contributions on mouseover, and also to sort the list dynamically (say, alphabetically within contribution bands) as people contribute.

Tracking the contribution types in the metadata file makes sense to me. Should we save v2 author ordering and mouseover contributions for a follow up issues/pull request?

content/08.methods.md

agitter · 2020-02-12T16:44:09Z

content/metadata.yaml

@@ -18,52 +18,60 @@ author_info:
    affiliations:
      - Molecular Biosciences and Bioengineering Graduate Program, University of Hawaii at Manoa, Honolulu, HI
    symbol_str: "☯"
+    v1: true


The convention you propose is that all v1 authors must make new contributions to be designated as v2 authors as well, right? Then at a later date we would decide how to merge v1 and v2 author lists?

I was thinking that we'd probably make the "v1" authors into a "boxed" author, but I don't have strong feelings on this. That approach most fits with current convention, but perhaps there is a better way.

So we'd add a consortium author like "Deep Review 1.0 Authors" (with a fancier name)? That makes sense to me. We can open an issue to discuss with the 1.0 authors once it advances that far.

Yea - this is the route I was imagining going.

agitter · 2020-02-12T16:52:25Z

In addition to denoting new authors, we should think more about how to denote new content with a better diff or some other annotation of Version 2.0 changes. If we release Version 2.0, does that by default imply that all sections are still current and accurate as of 2020? There have been noteworthy new ideas in most of the subsections we review. I don't think we'll be able to update them all.

@cgreene feel free to redirect this to another issue if more appropriate.

yfpeng · 2020-02-12T17:03:30Z

Is it possible that Version 2 only focuses on new content from 2018 to 2020? Many fields are progressing very fast.

delton137 · 2020-02-14T17:43:57Z

I am working on updating the parts on drug discovery, interpretability, and bias-variance tradeoff (including the phenomena of "double decent", which illuminates how deep nets function).

I feel the paper would benefit from an "executive copyedit" to bring coherence to the paper and also since some things are outdated. One major change is that many systems for medical imaging have reached radiologist-level accuracy and many are now approved by the FDA. (for a remarkable review and demonstration of this point, see this paper in The Lancet). The big challenge now, at least in medical imaging is "translation" to clinical use and ensuring trustworthiness and robustness. Much of the older work on visualizing and interpreting deep neural network function has gone by the wayside - people are realizing many of the interpretability methods are easy to misinterpret and not robust to small changes. There's interest in new paradigms like AIs that offer human understandable explanations as well as predictions, as well as a push for uncertainty quantification on predictions.

I could go on. However, I don't feel it's my place to do this high level copy edit, nor am I sure I would have the time.

akundaje · 2020-02-14T18:14:08Z

"Much of the older work on visualizing and interpreting deep neural network function has gone by the wayside - people are realizing many of the interpretability methods are easy to misinterpret and not robust to small changes. " - This is a common misconception and a false generalization based on studies performed on imaging data modalities. Many of these conclusions simply do not hold when applied to biological sequence inputs. I am happy to discuss this in detail. But I would strongly oppose any such comment in the paper without explicitly discussing the nuances of when these critiques apply.

akundaje · 2020-02-14T18:20:42Z

If any changes are being made to the interpretation sections, please give @AvantiShri and me a chance to reconcile the changes, since we largely wrote the original section. There are a lot of misunderstandings and disagreements about the efficacy of post-hoc interpretation methods across domains. So it is critical to make sure we reconcile these points with nuance.

delton137 · 2020-02-14T18:40:58Z

@akundaje I agree if it is re-written it needs to be balanced. Here's some papers to back up what I was saying. Rudin's work in particular has gotten attention in the medical imaging field, where saliency maps have become popular.

Rudin, C.: Stop explaining black box machine learning models for high stakes
decisions and use interpretable models instead. Nature Machine Intelligence 1(5), 206{215 (May 2019).

Dombrowski, A.K., Alber, M., Anders, C.J., Ackermann, M., Muller, K.R., Kessel,
P.: Explanations can be manipulated and geometry is to blame (2019)

Yeh, C.K., Hsieh, C.Y., Suggala, A.S., Inouye, D.I., Ravikumar, P.: On the
(in)delity and sensitivity for explanations. arXiv preprint: 1901.09392 (2019)

Obscure master's thesis which shows that layer-wise relevance propagation isn't very informative:
Lie, C.: Relevance in the eye of the beholder: Diagnosing classifications based
on visualised layerwise relevance propagation. Master's thesis, Lund Unversity,
Sweden (2019)

A recent article which attempted to give desiderata for interpretability is:
Murdoch, W.J., Singh, C., Kumbier, K., Abbasi-Asl, R., Yu, B.: Definitions,
methods, and applications in interpretable machine learning. Proceedings
of the National Academy of Sciences 116(44), 22071{22080 (Oct 2019).

Note I am working on a paper on "self-explaining" AI, as an alternative to trying to do interpretation to engender more trust. A very rough draft (which I admit needs much work) has been uploaded to the arXiv.

delton137 · 2020-02-14T18:44:05Z

Also, just to be clear, in my comment about stuff "falling to the wayside" that was my personal view upon reading the recent literature (especially Rudin's piece) and definitely not meant as a critique of what is in the paper.

dhimmel

approach looks good to me. I like the idea of putting author.approval_date or author.date_approved in metadata.yaml.

akundaje · 2020-02-14T19:49:07Z

Yes I've read these works especially Rudin's piece. Large parts of the critique simply do not apply to the discrete biological sequence domain where there is
(1) explicit often causal flow of information from input sequence to output phenotype
(2) phenotypes are often not binary or categorical eg. Complex profiles
(3) inputs are discrete sequences. Totally different compositionally compared to images
(4) Much shallower models
(5) in molecular biology we are particularly obsessed with confounders and experimental assays are explicitly used to measure biases which can be modeled. The computer vision and DL for medical imaging community have to deal with often unmeasured or immeasurable confounders.
(6) we can actually validate saliency methods by performing perturbation experiments on biological sequences and measure effects. You cannot perturb pixels in an image and perform an experiment to measure ground truth effects

Not a single example in that paper is obtained from biological sequence data. I agree with all the points in the context of the data types and domains that are being referred to.

It's extremely problematic to generalize papers from one domain to another without considering whether many of the issues fundamentally transfer over.

Also LRP is most certainly not a state of the art attribution method. And yes many previous saliency methods have been shown to be problematic eg. guided backdrop. But not all saliency methods are alike and they most certainly have very different behaviors across different input modalities.

So once again I'm not suggesting we don't highlight these issues in the domains in which they have shown to manifest. And I have no problem with critiques or edits or improvements to what we wrote.

My opposition was simply to the idea that we should simply dismiss post hoc interpretation altogether because some approaches fail or are unstable in specific domains.

No offense taken and none intended :)

delton137 · 2020-02-14T21:41:31Z

@akundaje I just finished reading the entire interpretability section to refresh my memory. I think it's very useful information and I haven't seen such a detailed review anywhere else (although a few papers come close). I submitted some changes to the introduction (which I'm now reconsidering), and left everything else intact. I think if we do decide to update this section further, the "future outlook" part is where we should focus first I think. Rudin's paper should be mentioned as well as the caveat you mentioned. Rudin (and others) do emphasize that this entire subject of interpretability (ie what counts as "useful") is domain specific (which leads to confusion).

cgreene · 2020-02-17T10:44:07Z

I am totally thrilled that this conversation is happening @akundaje @delton137, et al. On the other hand, this is a very procedural pull request just trying to figure out how we want to denote the various author contributions to multiple versions of the manuscript.

Could you discuss this in a separate pull request or issue? It seems like #985 might be the place to have a lot of the interpretability discussion.

AppVeyorBot · 2020-02-28T19:21:55Z

AppVeyor build 1.0.25 for commit a527157 by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

agitter · 2020-02-28T21:21:02Z

content/metadata.yaml

  - github: sw1
    name: Stephen Woloszynek
    orcid: 0000-0003-0568-298X
    email: sw424@drexel.edu
    affiliations:
      - Ecological and Evolutionary Signal-processing and Informatics Laboratory, Department of Electrical and Computer Engineering, Drexel University, Philadelphia, PA
+    v1: "Drafted one or more sub-sections."


Suggested change

v1: "Drafted one or more sub-sections."

v1: "Drafted one or more sub-sections."

coi:

string: "None"

last-approved: !!str 2017-05-26

agitter · 2020-02-28T21:22:29Z

content/metadata.yaml

@@ -204,25 +309,41 @@ author_info:
      - Department of Genetics, Stanford University, Stanford, CA
      - Department of Computer Science, Stanford University, Stanford, CA
    funders: NIH DP2GM123485
+    v1: "Revised specific sub-sections or supervised drafting one or more sub-sections."
+    coi:
+      string: "None"


Suggested change

string: "None"

string: "Advisory Board of Deep Genomics Inc."

agitter · 2020-02-28T21:23:19Z

content/metadata.yaml

  - github: enricoferrero
    name: Enrico Ferrero
    orcid: 0000-0002-8362-100X
    email: enrico.x.ferrero@gsk.com
    affiliations:
      - Computational Biology and Stats, Target Sciences, GlaxoSmithKline, Stevenage, United Kingdom
+    v1: "Drafted multiple sub-sections along with extensive editing, pull request reviews, or discussion."
+    coi:
+      string: "None"


Suggested change

string: "None"

string: "Full-time employee of GlaxoSmithKline"

AppVeyorBot · 2020-02-28T21:25:06Z

AppVeyor build 1.0.30 for commit e183c15 by @cgreene failed.

agitter · 2020-02-28T21:34:23Z

content/08.methods.md

+|Author|Competing Interests|Last Reviewed|
+|---|---|---|
+{% for author in manubot.authors %}
+|{{author.name}}|{{author.coi.string}}|{{author.coi.last-approved}}|


We'll have to change last-approved to last_approved here an in the metadata file. These are treated as Python variable names so they can't have - characters.

Do you prefer author names or initials in the table? We have access to both.

AppVeyorBot · 2020-02-28T21:35:13Z

AppVeyor build 1.0.31 for commit 3e649d9 by @cgreene failed.

AppVeyorBot · 2020-02-28T21:45:52Z

AppVeyor build 1.0.32 for commit 65bfcc9 by @cgreene failed.

agitter · 2020-02-28T21:51:34Z

content/08.methods.md

+Revised specific sub-sections or supervised drafting one or more sub-sections: .
+Drafted sub-sections, edited the manuscript, reviewed pull requests, and coordinated co-authors: C.S.G.
+
+{% for v2, authors in manubot.authors | groupby('v2') %}


I think this groupby is what is causing the build errors. The changes in 65bfcc9 may not be needed?

AppVeyorBot · 2020-03-02T15:49:45Z

AppVeyor build 1.0.41 for commit b5f65f4 by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

dhimmel · 2020-03-02T16:12:35Z

BTW builds of this PR will be slow until #992 is merged. We might want to merge that first.

cgreene · 2020-03-02T16:18:20Z

Wow - for my local builds this is substantially faster. 😮

AppVeyorBot · 2020-03-02T16:20:56Z

AppVeyor build 1.0.43 for commit 60b2712 by @cgreene is now complete. The rendered manuscript from this build is temporarily available for download at:

agitter

@cgreene merge whenever you're ready. I don't see any major issues. I'll do a detailed re-review post-merge and follow up with my own pull request if I see anything small.

I'm loving more automation for the contributions and COIs!

cgreene · 2020-03-02T18:24:50Z

🎉 yay! Automate everything!

[ci skip] This build is based on 5e9aa01. This commit was created by the following CI build and job: https://github.com/greenelab/deep-review/commit/5e9aa01dadad5f3cd87cd1f39dbe577dff651c64/checks https://github.com/greenelab/deep-review/runs/48332581

Update 00.front-matter.md

29b7ea5

cgreene added 2 commits February 10, 2020 14:10

try to split authors by version

066b3cc

update frontmatter notes

7facd07

cgreene added 3 commits February 10, 2020 14:20

add back symbols

d90da3d

note what v1.0 means

f36cf12

more clearly note what v2.0 means

8fb8b56

cgreene changed the title ~~[WIP] Update Underway~~ [Review Requested] Update Underway Feb 10, 2020

cgreene requested review from dhimmel and agitter February 10, 2020 19:22

cgreene mentioned this pull request Feb 10, 2020

Authors Since Publication #959

Open

3 tasks

create table for COIs with specific dates of approval (currently set …

cd2bfc6

…to most recent approval)

cgreene mentioned this pull request Feb 10, 2020

Section Status: Electronic health records #967

Open

agitter requested changes Feb 12, 2020

View reviewed changes

dhimmel approved these changes Feb 14, 2020

View reviewed changes

force not date

a527157

dhimmel mentioned this pull request Feb 28, 2020

YAML reader loads types that are not writable to JSON manubot/manubot#211

Open

does this work to make the COI table?

e183c15

agitter reviewed Feb 28, 2020

View reviewed changes

fingers crossed

3e649d9

agitter reviewed Feb 28, 2020

View reviewed changes

use false instead of undefined

65bfcc9

agitter reviewed Feb 28, 2020

View reviewed changes

auto-generated contribs

b5f65f4

Merge branch 'master' into cgreene-update-underway

60b2712

cgreene mentioned this pull request Mar 2, 2020

Note v1/v2 contributions on the header page. #993

Open

agitter approved these changes Mar 2, 2020

View reviewed changes

cgreene merged commit 5e9aa01 into master Mar 2, 2020

cgreene deleted the cgreene-update-underway branch March 2, 2020 18:25

cgreene mentioned this pull request Mar 5, 2020

add discussion in interpretability section and updates to molecular design section and discussion sections (issues with previous PR fixed) #988

Merged

agitter mentioned this pull request Mar 15, 2020

Minor changes for recent version 2.0 updates #1002

Merged

agitter mentioned this pull request Aug 15, 2020

Update my contributions to the interpretability section a bit #1020

Open

	string: "None"
	string: "Advisory Board of Deep Genomics Inc."

	string: "None"
	string: "Full-time employee of GlaxoSmithKline"

[Review Requested] Update Underway #986

[Review Requested] Update Underway #986

Conversation

cgreene commented Feb 10, 2020

AppVeyorBot commented Feb 10, 2020

AppVeyorBot commented Feb 10, 2020

cgreene commented Feb 10, 2020

AppVeyorBot commented Feb 10, 2020

AppVeyorBot commented Feb 10, 2020

AppVeyorBot commented Feb 10, 2020

agitter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agitter commented Feb 12, 2020

yfpeng commented Feb 12, 2020

delton137 commented Feb 14, 2020

akundaje commented Feb 14, 2020

akundaje commented Feb 14, 2020

delton137 commented Feb 14, 2020 • edited Loading

delton137 commented Feb 14, 2020

dhimmel left a comment

Choose a reason for hiding this comment

akundaje commented Feb 14, 2020

delton137 commented Feb 14, 2020

cgreene commented Feb 17, 2020

AppVeyorBot commented Feb 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AppVeyorBot commented Feb 28, 2020

Choose a reason for hiding this comment

AppVeyorBot commented Feb 28, 2020

AppVeyorBot commented Feb 28, 2020

Choose a reason for hiding this comment

AppVeyorBot commented Mar 2, 2020

dhimmel commented Mar 2, 2020

cgreene commented Mar 2, 2020

AppVeyorBot commented Mar 2, 2020

agitter left a comment

Choose a reason for hiding this comment

cgreene commented Mar 2, 2020

delton137 commented Feb 14, 2020 •

edited

Loading