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

Use latest dependencies in pipeline image #239

Merged

Conversation

lobis
Copy link
Member

@lobis lobis commented Jun 2, 2022

lobis Large: 1665

Closes #238.

The latest official root image uses Ubuntu 22.04 as base so I think we should use it as a base too, instead of Ubuntu 20.04. There may be some problems due to this such as cmake version changes etc.

In this PR I renamed a few files in pipeline/pandaxiii_MC. I noticed one of the files used as reference to check simulation (originally named validation.root I think) was taking too much space, around 25MB but only its first 10 events were being used. I reduced it in size saving around 23MB of space, but we would need to remove big blobs from the history doing a force push in order to reduce the repository size.

Related PR:

Part of this PR was merged in #248

@lobis lobis linked an issue Jun 2, 2022 that may be closed by this pull request
@jgalan
Copy link
Member

jgalan commented Jun 7, 2022

Now that it seems you are updating the docker image. It could be added -DSQL=ON as compilation option? This will require installing MySQL dev libraries (if not already available).

@lobis
Copy link
Member Author

lobis commented Jun 7, 2022

Now that it seems you are updating the docker image. It could be added -DSQL=ON as compilation option? This will require installing MySQL dev libraries (if not already available).

Sure, I can add it. I think these libraries are installed. I found no issue when enabling -DSQL=ON but no new target is being built, however I see:

===========================================================
System is: Linux
Packages: restG4 
Libraries: RestFramework;RestConnectors;RestDetector;RestGeant4;RestRaw
Features: Eve;SQL
===========================================================

Maybe I need -DREST_SQL=ON?

Feel free to add any compilation flags you want on this branch @jgalan I will install required dependencies if there is a problem.

@lobis lobis requested review from jgalan, juanangp and nkx111 and removed request for jgalan, juanangp and nkx111 June 7, 2022 13:52
@lobis lobis self-assigned this Jun 7, 2022
@lobis lobis requested a review from nkx111 June 7, 2022 13:52
@lobis lobis marked this pull request as ready for review June 7, 2022 13:57
@lobis
Copy link
Member Author

lobis commented Jun 14, 2022

I think this can be merged to fix pipeline @juanangp @jgalan. Gitlab pipeline has been updated to latest image version but the GitHub pipeline now also verifies examples run on old geant4 so I think we are covered.

@juanangp
Copy link
Member

I think this can be merged to fix pipeline @juanangp @jgalan. Gitlab pipeline has been updated to latest image version but the GitHub pipeline now also verifies examples run on old geant4 so I think we are covered.

Apologizes, I did a small mesh, I will accept as soon as I fix that. Btw, do you know why alphatrack validation is so ridiculous slow? It was not happening with previous geant4 version

@lobis
Copy link
Member Author

lobis commented Jun 14, 2022

I think this can be merged to fix pipeline @juanangp @jgalan. Gitlab pipeline has been updated to latest image version but the GitHub pipeline now also verifies examples run on old geant4 so I think we are covered.

Apologizes, I did a small mesh, I will accept as soon as I fix that. Btw, do you know why alphatrack validation is so ridiculous slow? It was not happening with previous geant4 version

Yes, its because of rest-for-physics/restG4#53, my guess is that some step limiters we are applying right now don't work on latest Geant4 versions and should be updated, however they give no errors. I think this should be fixable.

I also removed the uploading of the related artifact since it takes too much time / space but will add it back once this is fixed.

@lobis lobis requested a review from a team June 14, 2022 08:13
@juanangp
Copy link
Member

I also removed the uploading of the related artifact since it takes too much time / space but will add it back once this is fixed.

I agree, we can temporary avoid the upload of the artifacts till the issue is understood.

@juanangp
Copy link
Member

ce but will add it back once this is fixed

Doesn't looks like the artifacts are skipped in the latest pipeline, will try to fix it

@jgalan
Copy link
Member

jgalan commented Jun 14, 2022

I think ROOT validation files should never be modified/replaced. Perhaps adding new versions?

@jgalan
Copy link
Member

jgalan commented Jun 14, 2022

Probably ROOT validation files could be placed at an independent repository, and the files could be made public in a http location. Such as it is done with https://sultan.unizar.es/axionlib-data/ and the repository https://github.com/rest-for-physics/axionlib-data.

The http address is updated nightly in sultan through the rest user.

Then, the validation pipeline could just get the validation file needed using wget.

@lobis
Copy link
Member Author

lobis commented Jun 14, 2022

Probably ROOT validation files could be placed at an independent repository, and the files could be made public in a http location. Such as it is done with https://sultan.unizar.es/axionlib-data/ and the repository https://github.com/rest-for-physics/axionlib-data.

The http address is updated nightly in sultan through the rest user.

Then, the validation pipeline could just get the validation file needed using wget.

I agree but I think for the time being this PR could be merged to fix the pipelines.

Copy link
Member

@jgalan jgalan left a comment

Choose a reason for hiding this comment

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

I approve just to fix the pipelines, by this PR with 2000 lines changes, renaming of validation ROOT files, probably replacement. It is too much, thus imposible to review.

@jgalan jgalan self-requested a review June 15, 2022 07:47
@lobis lobis merged commit c67e803 into master Jun 15, 2022
@lobis lobis deleted the 238-update-pipeline-image-to-use-latest-version-of-dependencies branch June 15, 2022 07:58
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.

Update pipeline image to use latest version of dependencies
4 participants