Skip to content
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

Fars_vs_stat plot in results page #5052

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

rahuldhurkunde
Copy link
Member

@rahuldhurkunde rahuldhurkunde commented Feb 19, 2025

Incorporating the output of pycbc_page_fars_vs_stat plot in the pycbc results page. This plot would be visible in the summary page and under the background triggers page.

Standard information about the request

This is a: new feature which adds an additional plot to the summary page.

This change: has appropriate unit tests, follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines

Motivation

The far vs stat plots in the summary pages is crucial to validate the ranking of events during a offline run.

Contents

Adds a new job in the pycbc_make_offline_search_workflow which is defined in the workflow/plotting.py. Config changes in the mini-search to keep the changes consistent with this PR.

Links to any issues or associated PRs

The v2.3.10 release PR see (#5044) requires this to be merged first.

Testing performed

I've tested the mini-search workflow after updating the configs, and it runs successfully.

  • The author of this pull request confirms they will adhere to the code of conduct

@tdent
Copy link
Contributor

tdent commented Feb 20, 2025

From my point of view this is fine assuming it works and puts the plots in the right place ... Ian may have more to say on the workflow details

@spxiwh
Copy link
Contributor

spxiwh commented Feb 20, 2025

It does need to pass the workflow generation test ...

@spxiwh
Copy link
Contributor

spxiwh commented Feb 21, 2025

Failure is:

        RuntimeError: Failed to process string with tex because latex could not be found

@tdent
Copy link
Contributor

tdent commented Feb 21, 2025

I guess we should copy what other workflow plots do ?

@spxiwh
Copy link
Contributor

spxiwh commented Feb 21, 2025

@tdent @rahuldhurkunde I'm going to remove the "uselatex=True" line from the new plot here. This is in keeping with every other PyCBC code (bar one, which probably needs this as well). While the plot clearly looks better with latex, it does display fine without latex.

... I'm not particularly happy with this solution (because it means that in most cases we don't use latex where we could ... Although I remind that latex+matplotlib on clusters was always notoriously unstable and led to a lot of failures), but it is what is currently being used throughout the plotting suite. I think some common option to turn latex on, in every plotting code, would be welcome, but that's outside the scope of this PR. I note that one can use a matplotlibrc file to turn latex on by default for all jobs. So I'm going to "make this code do what all the other plotting codes do", and then leave the issue of using latex to a different issue/PR.

@spxiwh
Copy link
Contributor

spxiwh commented Feb 21, 2025

@tdent It would be good to confirm you are happy with removing latex before this is merged. I think it will pass the tests now within the next hour.

@tdent
Copy link
Contributor

tdent commented Feb 21, 2025

Uh, sure, I think I only inherited usetex from Gareth's pretty paper plot. I had no idea it might set off thoughts of globally reconfiguring the search pipeline plotting suite ...

@rahuldhurkunde
Copy link
Member Author

Failure while building the Docker container

ERROR: failed to solve: process "/bin/sh -c dnf -y install https://ecsft.cern.ch/dist/cvmfs/cvmfs-release/cvmfs-release-latest.noarch.rpm && dnf -y install cvmfs cvmfs-config-default && dnf clean all && dnf makecache &&     dnf -y groupinstall \"Development Tools\"                         \"Scientific Support\" &&     rpm -e --nodeps git perl-Git && dnf -y install @python39 rsync zlib-devel libpng-devel libjpeg-devel sqlite-devel openssl-devel fftw-libs-single fftw-devel fftw fftw-libs-long fftw-libs fftw-libs-double gsl gsl-devel hdf5 hdf5-devel python39-devel swig which osg-ca-certs && python3.9 -m pip install --upgrade pip setuptools wheel cython && python3.9 -m pip install mkl ipython jupyter jupyterhub jupyterlab lalsuite &&     dnf -y install https://repo.opensciencegrid.org/osg/3.5/el8/testing/x86_64/osg-wn-client-3.5-5.osg35.el8.noarch.rpm && dnf clean all" did not complete successfully: exit code: 1
Error: Process completed with exit code 1.

Checks in other recent PRs are instead building with https://repo.opensciencegrid.org/osg/3.5/el8/testing/x86_64/osg-wn-client-3.5-5.osg35.el8.noarch.rpm. Not sure why different OSG repos are appearing in the Dockerfile.

@spxiwh
Copy link
Contributor

spxiwh commented Feb 21, 2025

I'm not sure what you mean about OSG, that all looks the same to me. I think the error is:

144.6 Errors during downloading metadata for repository 'igwn-backports':
144.6   - Curl error (28): Timeout was reached for http://software.igwn.org/lscsoft/rocky/8/backports/x86_64/os/repodata/repomd.xml [Operation too slow. Less than 1000 bytes/sec transferred the last 30 seconds]
144.6 Error: Failed to download metadata for repo 'igwn-backports': Cannot download repomd.xml: Cannot download repodata/repomd.xml: All mirrors were tried

I suspect software.igwn.org has blocked github.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants