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.

Sign up for GitHub

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

Jump to bottom

Acc-Models Creation #432

Merged
merged 89 commits into from
Dec 19, 2023
Merged

Acc-Models Creation #432

merged 89 commits into from
Dec 19, 2023

Conversation

awegsche
Copy link
Contributor

This adds an overhauled modelcreation to omc3

Features:

  • useful help messages
  • can use different sources (afs, acc-models-like user defined path) for newer optics (LHC since 2018, injectors)
  • ability to query for optics files

@awegsche
try to print out some useful help messages
b2e5a34
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
2a14b04
@awegsche
add fetchers
61c603d 
- help message
- show modifiers
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
5b9a70a
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
78e7d5b
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
87fffe9
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
7436659
@awegsche
comments, formatting, add CreatedModel
d834c11
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
ab22144
@awegsche
comments, fix in CreatedModel
81b0953
@awegsche
start restructuring lhc_model_creator to reflect acc-models
a4c133e 
moving debug prints to logging
checked symlinking and folder creation
create empty knobs.txt at folder creation
@awegsche
add require_param and refactor model creation
b422e0c
@awegsche
also print available accelerators in help message
1a3408e
@awegsche
Refactoring model creation
66f94e0 
- bringing parts of the process into the right order
- fixing tests
- non-existent modifiers are caught earlier than before, this breaks tests (change?)
- remove some obsolete code
@awegsche
some comments
4f69d02
@awegsche
no silence
e720bf8
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
00e2c05
@awegsche
error type
fbab08e
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
f8213c2
@awegsche
add PSB model creation
07c9642
@awegsche
refactoring PS and PSB modelcreation to use acc-models
33af415
@awegsche
bugfixes,
bd7f74a 
- k1/kf
- clarification comments
@awegsche
Remove debug prints and wrong comments
cc863ad
@awegsche
formatting
00cad93
@awegsche
improve comments and structure of PS base mask
c7d382e
@awegsche
Remove some more debug prints
d6b835c
@awegsche
PsBase and PsBaseModelcreator
cedbc32
@awegsche
Merge branch 'master' into enhancement/292/modelcreation
1993827
@awegsche
query model creation args
aeef18a 
and update lhc model creator to use this
@awegsche
suppress output from base sequence loading in lhc
3c4b6c5
@awegsche
Copy link
Contributor Author

all green

tpersson
tpersson reviewed Dec 12, 2023
View reviewed changes
Copy link
Contributor

@tpersson tpersson left a comment

Choose a reason for hiding this comment

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

I cannot say much about the code. It looks good to me but I can't understand every detail of it without spending days on it. I just made 2 very minor comments. I think we really should get this merged in as soon as possible.

omc3/model/accelerators/accelerator.py Show resolved Hide resolved
omc3/model/model_creators/lhc_model_creator.py Show resolved Hide resolved
tpersson
tpersson previously approved these changes Dec 13, 2023
View reviewed changes
Copy link
Contributor

@tpersson tpersson left a comment

Choose a reason for hiding this comment

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

For me it looks very good and with all the test I think we can feel rather confident.

Thanks a lot for all this work Andreas.

@fsoubelet
Copy link
Member

Hi, sorry for the delay, I had a big deadline and presentation today.
I will review tomorrow, promise :)

fsoubelet
fsoubelet reviewed Dec 14, 2023
View reviewed changes
Copy link
Member

@fsoubelet fsoubelet left a comment

Choose a reason for hiding this comment

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

Thanks for the new model creation.

A few questions that came to mind when going through, and some comments here and there. You can consider these as suggestions more than anything.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
omc3/model/accelerators/accelerator.py Outdated Show resolved Hide resolved
omc3/model/accelerators/accelerator.py Outdated Show resolved Hide resolved
omc3/model/accelerators/accelerator.py Show resolved Hide resolved
tests/unit/test_accelerator_classes.py Show resolved Hide resolved
tests/unit/test_accelerator_classes.py Show resolved Hide resolved
tests/unit/test_model_creator.py Outdated Show resolved Hide resolved
tests/unit/test_model_creator.py Outdated Show resolved Hide resolved
tests/unit/test_model_creator.py Outdated Show resolved Hide resolved
@codeclimate CodeClimate
Copy link

codeclimate bot commented Dec 18, 2023

Code Climate has analyzed commit 4099470 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 84.6% (50% is the threshold).

This pull request will bring the total coverage in the repository to 86.0% (0.0% change).

View more on Code Climate.

@awegsche awegsche merged commit 89b01d6 into master Dec 19, 2023
38 checks passed
@fsoubelet fsoubelet deleted the model_creation_merged_with_master branch December 19, 2023 09:29
@JoschD JoschD mentioned this pull request Mar 13, 2024
Closed
@pylhctokens pylhctokens added the Type: Feature A (suggetion for a) new feature or enhancement in functionality. label Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fsoubelet fsoubelet fsoubelet approved these changes

@JoschD JoschD JoschD approved these changes

@tpersson tpersson Awaiting requested review from tpersson

Assignees

@awegsche awegsche

Labels
Priority: High Work on this! Type: Feature A (suggetion for a) new feature or enhancement in functionality. Type: Release Issue preparing for a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@awegsche @JoschD @fsoubelet @tpersson @pylhctokens