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

a few question and comments #115

Closed
zhaozhiwen opened this issue Oct 2, 2023 · 7 comments · Fixed by #122
Closed

a few question and comments #115

zhaozhiwen opened this issue Oct 2, 2023 · 7 comments · Fixed by #122

Comments

@zhaozhiwen
Copy link

zhaozhiwen commented Oct 2, 2023

I am testing the new code since 9/22 commit

One thing needs to change everywhere is NOT assuming "/w/hallb-scshelf2102/clas12/rg-b/offline_monitoring/clas12-timeline/outfiles/" as default dir so that files from different run group won't be mixed together. maybe the default should be just "./"

There are too many options now to run things.
It would be really helpful to have a short and concise instruction.
for example, here is what I figure out
******* my current quickstart for chef ****************************
module load clas12/pro
../../bin/run-monitoring.sh -d v1 --submit --focus-detectors -o ./ --rundir ../pass0/v29.38.1/mon/recon/
../../bin/run-detectors-timelines.sh -i timeline_detectors -d v1
mv /w/hallb-scshelf2102/clas12/rg-b/offline_monitoring/clas12-timeline/outfiles ./
../../bin/deploy-timelines.sh -d v1 -i outfiles/v1/timeline_web -t rgb/pass0
++++++++++++++++++++++++++++++++++++++++++++++++++++++

I see a bug as follows
clas12-1@ifarm1901 v1> ../bin/run-detectors-timelines.sh -i timeline_detectors -d v1 -o ./
../bin/run-detectors-timelines.sh: illegal option -- o

I see many difference for timeline made before and after 9/22 code. why is it?
https://clas12mon.jlab.org/rgb/pass0/v29.38.1/tlsummary/ (before) (without run 11252 on purpose)
https://clas12mon.jlab.org/rgb/pass0/v29.38.1_new/tlsummary/ (after) (with run 11252 on purpose)

@c-dilks
Copy link
Member

c-dilks commented Oct 2, 2023

I see many difference for timeline made before and after 9/22 code. why is it?
https://clas12mon.jlab.org/rgb/pass0/v29.38.1/tlsummary/ (before) (without run 11252 on purpose)
https://clas12mon.jlab.org/rgb/pass0/v29.38.1_new/tlsummary/ (after) (with run 11252 on purpose)

Can you give more details on what differences you see? I checked 3 timelines and see no difference, other than the missing run 11252.

These are RGB data, which is unfortunately the one run group whose detector timelines deviate from the rest, so you need to use the -r b option until resolution of #74.

@c-dilks
Copy link
Member

c-dilks commented Oct 2, 2023

Here's one difference: #116

@c-dilks c-dilks added this to the SWIF Integration milestone Oct 2, 2023
@c-dilks
Copy link
Member

c-dilks commented Oct 2, 2023

One thing needs to change everywhere is NOT assuming "/w/hallb-scshelf2102/clas12/rg-b/offline_monitoring/clas12-timeline/outfiles/" as default dir so that files from different run group won't be mixed together. maybe the default should be just "./"

This is a serious issue and I'll take care of it in a linked PR. Thanks for pointing it out.

@c-dilks
Copy link
Member

c-dilks commented Oct 2, 2023

All the extra options are primarily for developers and flexibility. Chefs will soon be re-trained to produce timelines in swif workflows, automatically producing them during usual cooking, so they should not need to worry about the extra options.

We're in the middle of this transition and will be testing this workflow on RGD data.

If you need to prioritize timeline production now, I recommend staying on v0.3 until things are more stable. The first version with swif integration will be v1.0, but likely will still have bugs until we've done more thorough testing.

@zhaozhiwen
Copy link
Author

Chris. I missed the option for rgb.
I will keep using the old version for now.
It's great if timeline would be part of workflow.

@c-dilks
Copy link
Member

c-dilks commented Oct 2, 2023

Chris. I missed the option for rgb.

I think the old version required this too.

Anyway, thanks a lot for the feedback and testing the new version!

@c-dilks
Copy link
Member

c-dilks commented Oct 5, 2023

One thing needs to change everywhere is NOT assuming "/w/hallb-scshelf2102/clas12/rg-b/offline_monitoring/clas12-timeline/outfiles/" as default dir so that files from different run group won't be mixed together. maybe the default should be just "./"

Fixed by #122

I see a bug as follows
clas12-1@ifarm1901 v1> ../bin/run-detectors-timelines.sh -i timeline_detectors -d v1 -o ./
../bin/run-detectors-timelines.sh: illegal option -- o

Fixed by #120

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

Successfully merging a pull request may close this issue.

2 participants