Skip to content

Presubmission Inquiry: pystiche #21

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

Closed
2 of 9 tasks
pmeier opened this issue Mar 10, 2020 · 22 comments
Closed
2 of 9 tasks

Presubmission Inquiry: pystiche #21

pmeier opened this issue Mar 10, 2020 · 22 comments

Comments

@pmeier
Copy link

pmeier commented Mar 10, 2020

Submitting Author: Philip Meier (@pmeier)
Package Name: pystiche
One-Line Description of Package: Framework for Neural Style Transfer
Repository Link (if existing): https://github.com/pmeier/pystiche


Description

  • Include a brief paragraph describing what your package does:

    pystiche is a framework for Neural Style Transfer (NST) algorithms based on PyTorch. NST is a neural-net-based technique to merge the content of one and the artistic style of another image. Similar to deep learning frameworks pystiche eases up the workflow for researchers in this field. Rather than implementing everything yourself, pystiche provides common building blocks of NST algorithms that can be conveniently combined. Thus, researchers can focus on implementing new ideas rather than implementing the periphery over and over again.

Scope

  • Please indicate which category or categories this package falls under:

    • Data retrieval
    • Data extraction
    • Data munging
    • Data deposition
    • Data visualization
    • Reproducibility
    • Geospatial
    • Education
    • Unsure/Other (explain below)
  • Explain how the and why the package falls under these categories (briefly, 1-2 sentences). Please note any areas you are unsure of:

    • Reproducibility: pystiche can be used to reproduce popular NST papers. I've already started this with pystiche_replication.
    • Unsure: IMO pystiche does not fit any other category. This is the reason for this presubmission inquiry.
  • Who is the target audience and what are scientific applications of this package?

    The primary intended audience are researchers as described above. Apart from them pystiche could also be interesting for recreational use by non-scientists due to its easy interface.

  • Are there other Python packages that accomplish the same thing? If so, how does yours differ?

    AFAIK there are no other packages provide a similar functionality. However, due to its popularity, there are many implementations, which are limited to a specific NST algorithm. An exception might be this which features the implementation of multiple papers.

  • Any other questions or issues we should be aware of?

    • Right now pystiche is still in an beta state and thus severely lacking documentation and tests. I'm actively working on this and thus this will change in next couple of months.
    • Regardless of the acceptance here, I plan to submit this to JOSS when finished.
@lwasser
Copy link
Member

lwasser commented Mar 26, 2020

hi @pmeier thank you for the submission and my apologies for the slow response. I will get back to this submission shortly after have a few others have a look! I appreciate your patience!

@pmeier
Copy link
Author

pmeier commented Apr 7, 2020

@lwasser I know this are difficult times right now, but could you give me an approximate timeline so I can plan around it?

@lwasser
Copy link
Member

lwasser commented Apr 7, 2020

hey @pmeier thank you for the checkin. Right now i'm trying to figure out whether we can review this package. I plan to bring this up at our meeting on thursday (which you are welcome to attend!) but we just need to ensure we can find reviewers for this package that know enough about deep learning to provide sound insight into the package functionality. Do you know of anyone that may be able to review this package and also does not have a conflict of interest ? In the meantime i am pinging the ropensci folks about this as well!

@pmeier
Copy link
Author

pmeier commented Apr 7, 2020

I plan to bring this up at our meeting on thursday (which you are welcome to attend!) but we just need to ensure we can find reviewers for this package that know enough about deep learning to provide sound insight into the package functionality.

I'll try to attend and answer possible questions from the team. If I don't make it feel free to ask them here.

Do you know of anyone that may be able to review this package and also does not have a conflict of interest ?

I'll ask around and get back to you on this.

@lwasser
Copy link
Member

lwasser commented Apr 7, 2020

Thank you @pmeier i have some more space now to work on this so i'll prioritize getting you a decision this week. Thank you for your patience. These times are tricky and many of us have found ourselves (myself included) behind on work tasks.

@pmeier
Copy link
Author

pmeier commented Apr 7, 2020

It was not at all my intention to rush anything. My only goal was to get an estimate when I can expect a decision. "In a month" would have been fine, although I of course prefer "at the end of the week".

@lwasser
Copy link
Member

lwasser commented Apr 7, 2020

@pmeier i completely understand! :) i just am recognizing how behind i am. i don't feel pressured by you one bit! I normally would have given you a time frame, up front! I hope to get back to you after our thursday meeting! I am waiting on feedback from the ropensci folks as well on how they handle ML / Deep learning types of submission. Thank you again for being so quick to respond AND patient as well!

@lwasser
Copy link
Member

lwasser commented Apr 9, 2020

We will accept this for review but will need to find reviewers with domain knowledge to asses s the "correctness" of implementation of algorithms in this package! we likely have a few weeks to do this.

@NickleDave
Copy link
Contributor

Hey @pmeier just wanted to echo @lwasser and say thank you for joining us for to the meeting today.

And ask you:
Do you know this review on NST?
https://arxiv.org/abs/1705.04058
https://ieeexplore.ieee.org/abstract/document/8732370
I remembered finding it before during a lit review.

The intro does a good job of emphasizing that computer vision has been working on style transfer long before people applied neural nets to the problem.
They also provide a repo with citations / links to implementations (maybe you have all these already)
https://github.com/ycjing/Neural-Style-Transfer-Papers

Maybe you could add a citation to your repo's README?
The link to the PyTorch docs definitely helps with context, but pointing to lit helps show it's an established research area.

I would've just raised an issue there, but also wanted to add this point ☝️ (that it is an established research area) re: the convo today

@pmeier
Copy link
Author

pmeier commented Apr 9, 2020

Do you know this review on NST?

Yes, I'm well aware of that work. The first draft came out around the time I started working on NST and this was a great help to me. If pystiche is "ready", I think it is a good idea to ping the authors and maybe get added to their list.

Maybe you could add a citation to your repo's README?

Sure. I wasn't aware you want to check out the project this soon 😉

[...] [NST] is an established research area

Define established for me. If you mean by established that there is a lot of research going on, I totally agree with you. But if you mean, by established that the field is already mature, I have to disagree. IMO there are too many big open questions (what is good for me and my PhD).

@pmeier
Copy link
Author

pmeier commented Apr 13, 2020

Hey @lwasser and @NickleDave

I hope I can discuss the overall structure of my project with you before I start the actual submission. Depending on the structure the testing and documentation effort and thus the time up to the submission will change significantly.

TL,DR

I have three main components:

  1. the framework that enables an easy implementation of NST algorithms,
  2. the reference implementation of NST papers with the above framework, and
  3. replication scripts that uses the reference implementation to reproduce the results from the papers.

I'm unsure what the best way of organizing them might be and would like to hear your opinion on this.


I've been flip-flopping when it comes to this since I first started pystiche. At first I kept all three components in a single project since back then they were not as separable as they are now. When this separability came I moved the reference implementation as well as the replication scripts to a another project. Now I had two installble packages, but a logical split between them.

In the current state I re-merged the reference implementations with the framework to only have a single installable package. The reference implementations for NST papers live in pystiche.papers while the scripts to replicate the results are implemented in pystiche_replication in another project.

With the pending submission to pyOpenSci I'm thinking that the second form (framework separated from reference implementation and replication scripts) might be the best since it clearly defines the scope of each project. Furthermore with this split I'm able to submit the framework to JOSS and the replication to another journal in the future.

Since this would be the third time I change the project structure, I think it is a good idea to ask for a second opinion. After the submission it will be a lot harder to make a change like this.

@NickleDave
Copy link
Contributor

Hey @pmeier I agree with you that it makes sense to split them to more clearly define the scope of the projects, and be able to submit a replication and a framework to appropriate venues.

For an example from outside deep learning world, see this Repro-pack from Barba group:
https://pyopensci.discourse.group/t/real-world-repo-packs-looking-for-examples/151/5?u=nickledave
The key finding of the paper is a result obtained with new functionality they added to their PyGBe library, even though JOSS had already reviewed the earlier version of PyGBe.

I also think scoping like this will actually help you get a better handle on what the key abstractions should be that pystiche offers, and disentangle them from the way they are used in any given paper. For example I see in one of your replications that the criterion is named by paper authors and year. This is good in terms of tying code to replications, but it's hard for a naive reader to know what's specific about the perceptual loss they propose (which I'm assuming is one of the core ideas of the paper) vs. any other perceptual loss. If you can name the criteria in a way that is more explicit, e.g. InceptionLoss or SSIMLoss, and then use those explicitly-named functions in your replications, it will simultaneously make the replications more readable, and make the framework more general. So I think decoupling the two now will give you much more freedom to do things like that.

@pmeier
Copy link
Author

pmeier commented Apr 18, 2020

Thanks for your input @NickleDave. I think I will move forward with the splitting it then.


If you can name the criteria in a way that is more explicit, e.g. InceptionLoss or SSIMLoss, and then use those explicitly-named functions in your replications, it will simultaneously make the replications more readable, and make the framework more general.

Actually that is already implemented as you proposed: the core components of pystiche are the pystiche.ops.Operators. Although usually the papers introduce only single new Operator, an NST algorithm combines multiple of them into a single optimization criterion. In the paper you mentioned the authors introduced the GramOperator, which is named after its functionality instead of its authors. The combined optimization criterion has no specific name in the literature and so I thought it would be the best to identify them by authors and year.

A user that simply wants to create a new Operator or combine existing ones in a novel way does not have to touch the implementations in pystiche.papers. On the other hand if the user explicitly wants to compare the performance of their implementation against existing ones, I think its fair that they know the paper they are comparing too.

Thoughts?

@NickleDave
Copy link
Contributor

NickleDave commented Apr 18, 2020

I see, thank you for helping me understand the code better--clearly I haven't read through it all yet.

Yes, the abstractions you pointed me to are exactly what I had in mind, and I think further illustrate the generality of the framework.

If you're going to split out the replications, not sure you want/need of my input on them, and maybe it's just a question of style, but: to me, naming a function by paper author / date still feels verbose. You already have the author / date as the sub-package name. Because you've already namespaced them this way, it's a bit redundant to then have things with names like ulyanov_et_al_2016.utils.ulyanov_et_al_2016_transformer when you could just as well write ulyanov_et_al_2016.utils.transformer. I think some of this might be necessary to avoid clashes with your parameter names (like, transformer)? But it seems like you could still use some naming convention across replications (e.g., just ulyanov_et_al_2016.core.training and johnson_alahi_li_2016.core.training instead of ...ulyanov_et_al_2016_training and ...johnson_alahi_li_2016_training) to leverage namespaces and keep things succinct.

But please don't let me be overly critical, since what really matters is you're doing the hard work of replicating.

@pmeier
Copy link
Author

pmeier commented Apr 18, 2020

[...] naming a function by paper author / date still feels verbose. You already have the author / date as the sub-package name. Because you've already namespaced them this way, it's a bit redundant to then have things with names like ulyanov_et_al_2016.utils.ulyanov_et_al_2016_transformer when you could just as well write ulyanov_et_al_2016.utils.transformer.

I think this a valid point. I will change that after the split.

But please don't let me be overly critical, since what really matters is you're doing the hard work of replicating.

Quite the opposite: if I would mind constructive criticism I shouldn't submit it to pyOpenSci (and work as a scientist in general IMO). If it is a matter of choice I can always put it on hold or reject your proposal and go with mine instead. But I think usually it will be the other way around: you look at this a user and with that different perspective you will see problems I completely overlooked or thought they have an obvious solution. So please: if you find the time, keep up the criticism until you have addressed everything that bothers you 😉

@NickleDave
Copy link
Contributor

lol ok you asked for it

looking forward to reviewing more as we go forward

@lwasser
Copy link
Member

lwasser commented Apr 20, 2020

hey @pmeier and @NickleDave i'm just catching up after a week offline. I totally agree with Dave's suggestions to split things out as it makes sense to have the more generalizable items in a package that pyopensci and joss supports and the research (re)implementations separate.

@pmeier
Copy link
Author

pmeier commented May 9, 2020

@lwasser @NickleDave

I've found two reviewers for technical side of pystiche. Should I introduce them here so you can get in touch with them?


Another question regarding the submission: am I allowed to work on pystiche while it is being reviewed? If yes, at what point should I start the submission? Imagine that I'm 60% finished with the documentation. Can I simply submit and add the remaining 40% while you are starting the review phase?

@lwasser
Copy link
Member

lwasser commented May 11, 2020

@pmeier thank you for finding reviewers. You should fully complete all intended work on pystiche prior to submitting it for full review.

@pmeier
Copy link
Author

pmeier commented May 11, 2020

@lwasser

thank you for finding reviewers.

So, should I introduce them here?

@lwasser
Copy link
Member

lwasser commented May 11, 2020

@pmeier i am so appreciative that you found reviewers!
let's do this

  1. please finish up all work on pystiche
  2. Then submit a full review issue. IN that issue please ping the 2 reviewers there AND myself and @NickleDave who will you the editor for this submission.

Does that work for you? i think that will be the most efficient. We close a presubmission issue once you have submitted a full review request so it's best if you ping them in the full review issue!
please say the word if you have any questions.

@pmeier
Copy link
Author

pmeier commented Jul 2, 2020

Submission in #25.

@lwasser lwasser moved this from Done to Closed in presubmission-inquiries Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

3 participants