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

Remove buildmaster #2032

Merged
merged 2 commits into from
Apr 3, 2024
Merged

Remove buildmaster #2032

merged 2 commits into from
Apr 3, 2024

Conversation

RoyStegeman
Copy link
Member

@RoyStegeman RoyStegeman commented Apr 2, 2024

Since we now moved to the new commondata format, we no longer need the buildmaster folder. The datafiles we need to keep have been moved to the nnpdf_data folder.

The first commit simply removes the buildmaster folder
The second commit removes some outdated parts from the docs

In general I noticed the docs have a lot of outdated info (mostly old pipeline) in them that we should either update or remove. That's not the point of this PR though.

Copy link
Member

@comane comane left a comment

Choose a reason for hiding this comment

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

Hi @RoyStegeman thanks for this, this looks good to me.

I just noticed that in two of the jets datasets I had re implemented
eg nnpdf_data/nnpdf_data/new_commondata/ATLAS_2JET_7TEV_R06/filter.py

there are still mentions of buildmaster. This was for the purpose of testing and has unfortunately not been removed.

In the if __name__=="__main__" of these filter files I think one should only keep the parts that generate the datasets yaml files and remove the rest.

But I guess that this could in principle be done in a separate PR and I could take care of this.

@RoyStegeman
Copy link
Member Author

I was aware of that (also some of the imports of those files are broken), but as you say, those should be fixed in a different PR.

Instead of removing the references to the old commondata files it may also be an option to point the paths to the nnpdf_data/commondata folder, I'm not sure what the best thing is to do there. It's great you want to take care of that!

You may remember that during a recent CM we discussed exactly this - that the filter.py files have not been properly reviewed and many of them are or were broken. We would like to fix this, and, where possible, make files that share the same utils, take them from a single location (as has been done in #2021 for some datasets) to make future reviews easier. At the time you offered to take charge of organising/fixing those files, is that still something you're planning to do?

@RoyStegeman RoyStegeman merged commit fabef9b into master Apr 3, 2024
6 checks passed
@RoyStegeman RoyStegeman deleted the remove_buildmaster branch April 3, 2024 22:14
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.

None yet

2 participants