Factor out visualisation code

[eddieschoute]

Current issue

I am trying to install qiskit with pypy (v.6.0.0) but it fails on the matplotlib dependency. I don't need matplotlib in my project as a runtime dependency since I am only generating dataframes (from pandas). Later, I use these dataframes in a python notebook to do visualisation.

What is the expected enhancement?

Currently, qiskit-sdk depends on two visualisation libraries: matplotlib and pillow. These do not provide core functionality for qiskit, but rather provide a nice way for a user to interact with the data structures provided by qiskit. I suggest factoring out the visualisation code and dependencies into a separate project or plugin.

Furthermore, it may be worth it to re-evaluate which dependencies provide "core" functionality for qiskit as a library rather than as an application.

[ajavadia]

I don't object to making the visualization a plug-in. Although, in reality, Qiskit is less used as a library and more as a tool for working with quantum information (by the vast majority of users). And taking matplotlib out means you can't even see histograms of your runs.

But, based on your comment on Slack about C bindings, are you suggesting keeping Qiskit purely Python? I can say this will be problematic, because components of Qiskit are going to gradually move to C++ for performance. Not sure if this is what you meant though.

[eddieschoute]

An end-user of qiskit could, for example, install the qiskit-visualazation package that would depend on qiskit-core and thus bring in all dependencies. I don't think users that use qiskit as tool would notice much of a difference.

I did not suggest getting rid of C bindings.

[ajavadia]

Ok. This sounds good, to reduce dependencies of Qiskit on external libraries and make it leaner.

[eddieschoute]

Yes, I think qiskit could function both as a library and an application. But the library should have minimal dependencies.

[nonhermitian]

How about keeping the core package useful, and just have the visualization module dependencies optional, and only loaded if the visualization functions are called?

On Wed, Jun 27, 2018, 22:14 Eddie Schoute notifications@github.com wrote:

Yes, I think qiskit could function both as a library and an application. But the library should have minimal dependencies.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/QISKit/qiskit-core/issues/595#issuecomment-400814268, or mute the thread https://github.com/notifications/unsubscribe-auth/ABMPqUsnJhR3z_yBH0H44Q7faCRg-S4Lks5uA-e5gaJpZM4U2gWz .

[eddieschoute]

I don't see how a qiskit-core without matplotlib would be "useless"?

Plus, you cannot require new dependencies at runtime. There is, however, support for optional dependencies called "extras": https://setuptools.readthedocs.io/en/latest/setuptools.html#declaring-extras-optional-features-with-their-own-dependencies You could simply throw an error if a user tries to use a module without the appropriate extra installed. This would be somewhat more modular, but also more involved for the user.

[jaygambetta]

@eddieschoute it is our plan to make the visualizations a plugin but its not the top of the priority. Everything in the tools directory will be elements of qiskit over time. Acqua started with removing the optimization.

[nonhermitian]

Visualization of circuits is kind of a key thing to have in a package that people use for building quantum circuits/algorithms. I use it nearly every day. The visualization tools and/or matplotlib are used in nearly every tutorial notebook QISKit has. The vast majority of people find it useful. I don't see why features need to be stripped when simple alternatives such as import at function call exist. I would say that, at least in the near term, visualization is a key part of the core library, just not part of the functionality that you need.

[eddieschoute]

I don't see why features need to be stripped when simple alternatives such as import at function call exist.

They shouldn't be stripped. But moved to a separate package with appropriate dependencies so that dependencies remain consistent and predictable. Runtime errors are very annoying.

[jaygambetta]

yes paul but its not going to be factored out of pip install qiskit. As i said i want to change qiskit-core to qiskit-terra and for eddie clearly he only wants to install terra for most users they use all of qiskit

[jaygambetta]

This i leaving on hold until we have time to think about it more.

[mtreinish]

I think visualization is a key function of terra. At least for the foreseeable future (it might change eventually) so I don't think we should not remove the external dependency on matplotlib, etc. So I'm going to close this for now.

That being said, I think one thing we can look at longer term after we add the ascii drawer in #1054 and make the changes to remove all the paths we deprecated added in #1055 is using setuptools extras to make matplotlib an optional requirement. So on a default pip install qiskit we won't have matplotlib installed, which would only be fine if none of the default visualizations use it. But then people could do pip install 'qiskit[extra_graphs]' and get matplotlib and have the things that require it work. But that's something to look at in the medium to long term and we should probably open a separate issue to discuss that approach when the time is right.

[1ucian0]

I think this needs to be reopened because the @eddieschoute's point still stands.

It can be closed when matplotlib and pillow get removed from requirements.txt.

[mtreinish]

@1ucian0 no, I was saying that we won't ever be factoring the visualization code into a separate package or an optional plugin since it's a core function of terra today (which aligns with what @nonhermitian was saying earlier). It's tightly coupled with user expectations on what terra provides today and the only case in which we could move the code externally would be as a hard requirement for terra (which would not fix the primary point here about dependencies). So we're not going to be removing the visualization libraries from the requirements anytime soon because we're not going to be removing the drawing code from the terra repo/package.

The second paragraph was more me musing at an alternative approach we could consider longer term, but that's not actually achievable in any definable time frame. Landing #1054 is just an early prerequisite to even begin to consider what I was talking about there, it's not actually something actionable after #1054 lands (especially because the non-circuit drawer functions like bloch sphere, histogram, etc still depend on matplotlib).

Basically I closed this as invalid/won't fix, not that we've fixed it.

[1ucian0]

I see the original post as "remove the visualization dependencies", while the "refactor out" is "the suggested solution". I understand that we are not going to take that "suggested solution" (even when I don't agree). But we are going to solve the issue with respect to the visualization dependencies. So, it can be closed when fixed.

Did I understand you correctly @eddieschoute?

[mtreinish]

@1ucian0 except that's not actually an option. We have hard import dependency on the external libraries because matplotlib is used in many places. Unless we remove these hard dependencies from ever being imported at the module level (which we cannot do as I explained above, it's not just the circuit_drawer) they will have to be listed in the requirements for the terra package. There is not really a middle ground here where we remove dependencies and keep the code which requires those libraries in terra. Especially moving forward if we're considering having visualization as a top level thing in the terra tree, and/or adding a draw() method to top level constructs like QuantumCircuit.

As I said earlier in my first post on this issue I was thinking out loud about a potential approach we could consider takin at some indeterminate point in the future. But it's not actually something we can do now or any time soon unless we come up with alternative default backends with no external dependencies for all the visualizations we support or drop functionality that we both agree is core to terra. It's disingenuous to keep this issue open on just that, it's not actually something anyone is going to be working on or merging into terra.

[eddieschoute]

I think what I consider to be "foundations of qiskit" (as implied by "qiskit-terra") do not align with @mtreinish 's views. To me, the core of qiskit is quantum information processing. Providing tools to work with quantum circuits. And, possibly, running circuits via simulations or on quantum devices. Simply removing functionality is not my goal (the impact of the plotting code in qiskit itself is insignificant), but rather removing dependency bloat. Graphing is not necessary for using qiskit, which is why my suggestion could be feasible. I think matplotlib and similar dependencies are not insignificant and to make them mandatory should be considered carefully. Therefore, I would be interested to see why @mtreinish thinks these are core to qiskit (and therefore qiskit-terra).

Simply removing them from the requirements will cause errors if the user does not install the appropriate package and should be avoided. This is going to happen nonetheless if you go the way of pip extras, albeit with a nicer (custom) error message, perhaps.

[mtreinish]

@eddieschoute I feel it's a key part of qiskit because at least for today visualizing your results and/or the input circuits are a key part of how most people are using qiskit. You can look to what @nonhermitian said a few months ago, at most of the papers out there citing that they use qiskit, or most of the qiskit-terra examples out there and the majority of those either draw the circuits or visualize the results from the experiments/jobs in some way. Also moving forward on the plan for the next release we're looking at things like integrating visualization methods into the key primitives in terra. Like adding a draw() method on the quantumcircuit class (see #911 for that). Examples of that will likely increase in the future too. I could also make the argument that lots of functions in terra aren't necessary for a particular individual use case. You may not personally need graphing for how you use terra but that doesn't exclude that others view it as a key feature of qiskit. We can argue about the semantics of "foundations of qiskit" and what that really means and just honestly disagree on this point though.

Which is really how I'm looking at this, from a more pragmatic software maintenance point of view. We have a defined public API with an implicit contract for doing visualizations in terra today. Simply removing it, making it optional and opt-in (via setuptools extras or some other mechanism), shifting it into a separate optional qiskit-foo python package, or any other option which removes matplotlib from requirements.txt (and the requirements list in setup.py) or as an implicit dependency that gets installed by default will change that api contract in a way which will break existing users. We already have enough breaking API changes today and we're getting better about it and don't want to do this. So doing this without a proper deprecation period that emits a warning of the coming change is not really an option. That being said to do something like that has a very high cost and we have to have a very good reason for it, and I think that just removing matplotlib from the requirements list isn't sufficient to justify it. Especially because I think there isn't a clear consensus that graphing isn't a key function of terra.

This all being said I do agree with you on the cost of having matplotlib as a required dependency can be high for certain environments (mainly on osx and pypy). But drawing the line at matplotlib does seem a bit arbitrary to me. From what I can tell matplotlib has been in the requirements to qiskit for as long as we've supported using pip (which predates my work on the project) and it's also far from being the most difficult package to use in terra's requirements file. Both scipy and numpy are in the requirements file (and used in parts of qiskit that I think we both agree are core) and those are much more difficult to install properly and get running. Especially, under pypy (which fwiw we don't list as a supported python interpreter for this reason) where as far as I know scipy still doesn't work. For numpy I think you can get it working, but you need to jump through some hoops to get there. Also at least according to https://morepypy.blogspot.com/2018/04/pypy27-and-pypy35-v60-dual-release.html you can use matplotlib under pypy with the tkagg backend.

[nonhermitian]

The treatise by @mtreinish spells it out nicely.

[eddieschoute]

I agree with you, especially on the software maintenance point. I don't think it is necessarily impossible: Introducing a qiskit-graphing package with a dep on qiskit-terra that hooks draw() calls into the appropriate classes, or provide a polymorphic draw function, could solve these issues. But it certainly would break some code. I guess that's why I put it forward, now that qiskit it still in <1.0 stage. Anyhow, I think it is completely fair to decide that the development pains exceed the gains.

On an unrelated note: I personally decided not to use pypy after some profiling a while ago and thus do not have a need for this any more at this moment.

[ajavadia]

I think it is decided that visualizations will stay in Terra for the time being. We have relaxed the reliance on Latex and Matplotlib (basically you can get by without them to draw circuits), and matplotlib will only be imported when explicitly requested (#1275).