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

add benchmarks/u_rho #3

Draft
wants to merge 173 commits into
base: master
Choose a base branch
from
Draft

add benchmarks/u_rho #3

wants to merge 173 commits into from

Conversation

zsweger
Copy link

@zsweger zsweger commented Jan 13, 2024

Briefly, what does this PR introduce?

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No breaking changes.

Does this PR change default behavior?

This PR should create an automatic benchmark using u-channel rho production to benchmark the tracking performance of the B0 trackers

@zsweger zsweger marked this pull request as draft January 13, 2024 21:02
@zsweger
Copy link
Author

zsweger commented Jan 13, 2024

  • I would like to make this into one of the automatic benchmarks which runs full simulations when there are detector or tracking updates, and then makes the plots which I've written here.
  • What I've written now runs over reconstruction files from one of the simulation campaigns.
  • Can the experts please help me dit this benchmark to automatically run the simulations and then process the results?
  • Also I have the benchmark produce some plots in the u_rho/figures directory. How can I convert these into artifacts?

@veprbl
Copy link
Member

veprbl commented Jan 13, 2024

Current config.yml is suitable for detector_benchmarks, but not physics benchmarks:
https://eicweb.phy.anl.gov/EIC/benchmarks/physics_benchmarks/-/pipelines/78788
image
You'd need to look at https://github.com/eic/physics_benchmarks/blob/master/benchmarks/diffractive_vm/config.yml for reference, but it's at least a matter of replacing .det_benchmark with .phy_benchmark.

@veprbl
Copy link
Member

veprbl commented Jan 13, 2024

Producing artifacts requires placing your output files to results directory. I think I haven't implemented this for diffractive_vm yet. Ideas are welcome.

@zsweger
Copy link
Author

zsweger commented Jan 13, 2024

The config.yml file has been updated for the physics benchmarks

@zsweger
Copy link
Author

zsweger commented Jan 13, 2024

Producing artifacts requires placing your output files to results directory. I think I haven't implemented this for diffractive_vm yet. Ideas are welcome.

Sorry for the naïve questions, but does this just mean I should rename the figures directory to results?

@veprbl
Copy link
Member

veprbl commented Jan 13, 2024

Producing artifacts requires placing your output files to results directory. I think I haven't implemented this for diffractive_vm yet. Ideas are welcome.

Sorry for the naïve questions, but does this just mean I should rename the figures directory to results?

Yes.

stage: collect
needs: ["u_channel_rho:simulate"]
script:
- cp benchmark_output/campaign_23.12.0_combined_60files_eicrecon.edm4eic.plots_figures/*.pdf figures/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One issue here is that the contents of the "current" directory is not preserved, only what's in the $LOCAL_DATA_PATH space. I think it would be simpler to move this line to the line after you call snakemake, and let gitlab output results/ directory as an artifact.

@zsweger
Copy link
Author

zsweger commented Jan 13, 2024

Okay I edited the procedure to immediately create the results directory and copy to it.

Zachary W Sweger and others added 7 commits January 13, 2024 18:45
Testing removing analysis compilation from u-channel rho pipeline
Reimplemented compilation step
Updated compile step in snakefile
Edited compile rule
Updated file paths
Updated compile path
@zsweger
Copy link
Author

zsweger commented Jan 14, 2024

@veprbl , @wdconinc I don't know why this keeps failing at the compile step of the pipeline. Could you offer any insight?

@veprbl
Copy link
Member

veprbl commented Jan 14, 2024

Should be a matter of including into the top-level Snakefile. I've pushed the changes, let's see if it works.

@veprbl
Copy link
Member

veprbl commented Jan 14, 2024

@zsweger
Copy link
Author

zsweger commented Jun 7, 2024

Understood. I've checked few things, but nothing stands out. Could you file this bug to https://github.com/eic/EICrecon/issues, please.

Done! eic/EICrecon#1500

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

Successfully merging this pull request may close these issues.

3 participants