Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-14159

Wrap matplotlib use in meas_mosaic

    XMLWordPrintable

    Details

      Description

      meas.mosaic.utils does a module-level import of matplotlib and sets the backend to Agg.

      a) Specifically the matplotlib.use("Agg") line should be removed.

      It has no effect if the user has already loaded a backend, and spews a big chunk of distracting warnings.

      b) The overall import of matplotlib should not happen at the module level, because that code gets run when importing the module.  Importing the module happens even just opening a Butler object view to the repo.  Specifically, meas_mosaic gets imported by just the following code.

      import os
      import lsst.afw.display as afwDisplay
      from lsst.daf.persistence import Butler
       
      repo = os.getenv('REPO')
      butler = Butler(repo)
      

      Philosophically I don't believe that opening a Butler should trigger setting up matplotlib.

      Editorial: I believe that all plotting imports should be protected and separated from the data analysis.  But this implies a refactoring that may not be justified given the imminent replacement by jointcal

        Attachments

          Issue Links

            Activity

            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            And to clarify, what really surprised me about this is that I'm not consciously using meas_mosaic and I can't find where it gets picked up.  Why does it get imported anyway?

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - And to clarify, what really surprised me about this is that I'm not consciously using meas_mosaic and I can't find where it gets picked up.  Why does it get imported anyway?
            Hide
            price Paul Price added a comment -

            Possibly here?

            Show
            price Paul Price added a comment - Possibly here ?
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            +1

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - +1
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            Other code that uses matplotlib on machines without X (e.g. the worker nodes) falls over because of this.

            Basically, because one ends up not being able to set the backend (because the matplotlib.use('Agg') call fails to have an effect) any calls to make figures or plot things results in it trying to contact the display server and failing (with an ugly an non-obvious exit).

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - Other code that uses matplotlib on machines without X ( e.g. the worker nodes) falls over because of this. Basically, because one ends up not being able to set the backend (because the matplotlib.use('Agg') call fails to have an effect) any calls to make figures or plot things results in it trying to contact the display server and failing (with an ugly an non-obvious exit).
            Hide
            mfisherlevine Merlin Fisher-Levine added a comment -

            For others suffering, a temporary workaround is to put
            backend : Agg
            in your matplotlibrc file.

            See instructions here for how to do so:

            https://matplotlib.org/users/customizing.html#the-matplotlibrc-file

            Show
            mfisherlevine Merlin Fisher-Levine added a comment - For others suffering, a temporary workaround is to put backend : Agg in your matplotlibrc file. See instructions here for how to do so: https://matplotlib.org/users/customizing.html#the-matplotlibrc-file
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Merlin Fisher-Levine

            If the above worked for you, then it's not the meas_mosaic matplotlib.use statement in the module import that's causing you problems, it must be something else that's setting the matplotlib backend to a default that's not available on the worker nodes.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Merlin Fisher-Levine If the above worked for you, then it's not the meas_mosaic matplotlib.use statement in the module import that's causing you problems, it must be something else that's setting the matplotlib backend to a default that's not available on the worker nodes.
            Hide
            Parejkoj John Parejko added a comment - - edited

            Paul Price We really shouldn't have meas_mosaic dependencies in pipe_tasks, particularly ones that aren't captured in the table file. What is that doing there?

            Show
            Parejkoj John Parejko added a comment - - edited Paul Price We really shouldn't have meas_mosaic dependencies in pipe_tasks, particularly ones that aren't captured in the table file. What is that doing there?
            Hide
            price Paul Price added a comment -

            How do you expect us to use it if we can't use it?

            Show
            price Paul Price added a comment - How do you expect us to use it if we can't use it?
            Hide
            Parejkoj John Parejko added a comment - - edited

            Isn't pipe_tasks below meas_mosaic in the stack hierarchy? This seems like a recipe for circular dependencies.

            To actually answer your question: code that does the "apply wcs/photoCalib" step (in this case applyMosaicResults) should just live in pipe_tasks or even lower down (pipe_base?). It shouldn't have any dependency on meas_mosaic or jointcal, and it should cleanly handle the non-existence of those datasets by doing a no-op.

            Show
            Parejkoj John Parejko added a comment - - edited Isn't pipe_tasks below meas_mosaic in the stack hierarchy? This seems like a recipe for circular dependencies. To actually answer your question: code that does the "apply wcs/photoCalib" step (in this case applyMosaicResults ) should just live in pipe_tasks or even lower down (pipe_base?). It shouldn't have any dependency on meas_mosaic or jointcal, and it should cleanly handle the non-existence of those datasets by doing a no-op.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            This specific issue addressed by this ticket doesn't cause any significant problems.  It's just annoying to see the warning.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - This specific issue addressed by this ticket doesn't cause any significant problems.  It's just annoying to see the warning.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment - - edited

            `pipe_tasks` is "stuff we do with the pipeline".

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - - edited `pipe_tasks` is "stuff we do with the pipeline".
            Hide
            Parejkoj John Parejko added a comment -

            I don't believe the fix on the ticket branch will solve the related problem here, which is that if this code was imported one would still not be able to set the matplotlib backend because of the subsequent import matplotlib.pyplot as plt.

            Show
            Parejkoj John Parejko added a comment - I don't believe the fix on the ticket branch will solve the related problem here, which is that if this code was imported one would still not be able to set the matplotlib backend because of the subsequent import matplotlib.pyplot as plt .
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Good point.  Considering.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Good point.  Considering.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            After some discussion with John Parejko, it became clear that the correct way to solve this for validateDrp.py example runs would be to specify the backend in a current-working directory level matplotlibrc file. This has the benefit of removing hard-coded lines that might not even work and making it more clear where this is happening for the runs that we'd like to distribute to displayless nodes.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - After some discussion with John Parejko , it became clear that the correct way to solve this for validateDrp.py example runs would be to specify the backend in a current-working directory level matplotlibrc file. This has the benefit of removing hard-coded lines that might not even work and making it more clear where this is happening for the runs that we'd like to distribute to displayless nodes.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment - - edited

            After further discussion it may be that putting these lines in bin.src/validateDrp.py is a better and more clearer solution.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - - edited After further discussion it may be that putting these lines in bin.src/validateDrp.py is a better and more clearer solution.
            Hide
            Parejkoj John Parejko added a comment -

            One comment about your comment, otherwise you're good to go.

            Show
            Parejkoj John Parejko added a comment - One comment about your comment, otherwise you're good to go.
            Hide
            wmwood-vasey Michael Wood-Vasey added a comment -

            Merged to master.

            Show
            wmwood-vasey Michael Wood-Vasey added a comment - Merged to master.

              People

              Assignee:
              wmwood-vasey Michael Wood-Vasey
              Reporter:
              wmwood-vasey Michael Wood-Vasey
              Reviewers:
              John Parejko, Simon Krughoff
              Watchers:
              John Parejko, Merlin Fisher-Levine, Michael Wood-Vasey, Paul Price, Simon Krughoff
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.