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

batch 1 for updates to LST integration in cms-sw #34

Conversation

VourMa
Copy link
Collaborator

@VourMa VourMa commented Jun 5, 2024

accumulated list of features in the batch:

  • Initial commit to update directory for LST data files. Maybe it makes sense to merge this quickly to see if we can get the bot tests running successfully.
  • fixes in standalone from Standalone fixes #33 for a couple of things on the standalone version. It fixes the setup script and makefiles so that it can be built in standalone mode without any extra steps. It also fixes an issue with the ROOT file that the standalone version creates since it saves the git logs, but the logs of the CMSSW repo are too large now, so they are now truncated to the 100 latest commits.
  • Remove outdated files
  • fix loading of lstModulesDevESProducer by attaching it to an unused task by default to get the framework to skip it (not construct) when LST is not used. This fixes unit tests failing with Unable to find plugin 'LSTModulesDevESProducer@alpaka'

@slava77 slava77 changed the title Update directory for LST data files batch 1 for updates to LST integration in cms-sw Jun 5, 2024
@slava77
Copy link

slava77 commented Jun 6, 2024

cms-data should be in the 2300 IB today.

While I did not get a response from Matti on the fix proposed in #35 , the comments in the cms-sw PR were in line with this solution be more preferred than the others (accelerators_cff or makeProcessModifier).

So, I wanted to propose to add #35 to this "batch 1".

We can already merge this "batch 1" into the cms-sw PR some time as early as this evening CA time.

Anything else we expect to have ready by then or if need to wait, by the meeting time tomorrow?

@VourMa
Copy link
Collaborator Author

VourMa commented Jun 6, 2024

So, I wanted to propose to add #35 to this "batch 1".

Fine by me.

We can already merge this "batch 1" into the cms-sw PR some time as early as this evening CA time.

Fine by me as well.

The only additional updates I can push right now is the deletion of the (outdated) test scripts and the (outdated) README. Other than that, I wanted to deal with some of the first comments but I am not sure I will manage to do it today/early tomorrow, and they are not super important for now.
All in all, give me a couple of hours to clean up a couple of files and then you can merge from my side. I can ping you when I am done.

@VourMa
Copy link
Collaborator Author

VourMa commented Jun 6, 2024

@slava77 I am done for this batch.

@@ -1,42 +0,0 @@
# LSTCore proof of concept
Copy link

Choose a reason for hiding this comment

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

a readme may be still useful for self-documentation. The content removed here is probably good to go, but we need to eventually add something up to date.

The standalone/README is outdated as well; another to-do "soon"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the general README, let's discuss the content tomorrow at the meeting.

For the standalone one, I can draft an update when I port my SegmentLinking/TrackLooper#408 PR in the cmssw repo and run everything, and Andres can finalize with the details of the CI when these are settled.

@slava77 slava77 merged commit 9f697fd into CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles Jun 6, 2024
@slava77 slava77 deleted the CMSSW_14_1_0_pre3_LST_X_LSTCore_realfiles_integration branch June 6, 2024 23:48
@VourMa VourMa mentioned this pull request Jun 7, 2024
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.

4 participants