search

XENONnT / straxen

Streaming analysis for XENON
BSD 3-Clause "New" or "Revised" License
20 stars 32 forks source link

automatically appending local rucio path #1182

Closed FaroutYLq closed 1 year ago

FaroutYLq commented 1 year ago

What does the code in this PR do / what does it improve?

Liberate users from manually adding midway rucio RSE by specifying 

st_midway = cutax.xenonnt_online(_rucio_local_path='/project/lgrandi/rucio', include_rucio_local = True)

Can you briefly describe how it works?

Introduced a new parameter in xenonnt_online called _auto_append_rucio_local. If True (by default it is True), it will make include_rucio_local = True and append the midway rucio path in _rucio_local_path.

Can you give a minimal working example (or illustrate with a figure)?

After modification, st = cutax.xenonnt_online() will have access to data in midway rucio if you are not on dali compute nodes.

Please include the following if applicable:

FaroutYLq commented 1 year ago

This is just a dirty workaround. The proper way to do it will require decoupling hardcoding in straxen.RunDB. For example: here.

FaroutYLq commented 1 year ago

Meanwhile, we should probably also refactor _rucio_local_path. It has been marked "Only use for testing!" for a while...

coveralls commented 1 year ago

Coverage Status

coverage: 93.459% (+0.04%) from 93.417% when pulling 00d40488d4173f363d171f60d7cf84f070271e0b on which_rucio into f39d8ead55b0dbc5d7f27012e1c9d943dec72983 on master.

dachengx commented 1 year ago

Whether we want to merge it like this depends on whether we want to postpone the complete decoupling of dali in straxen's hardcode.

FaroutYLq commented 1 year ago

Since we are having tutorial for users to load data now, I feel less urgency to do this dirty workaround. Personally I suggest in straxen we do things technically better rather than quick dirty tricks like what I did. I would close it for now and let's pick up once we have more elegant solution (and time : )). Thanks @dachengx

FaroutYLq commented 1 year ago

@dachengx . I discussed with @jingqiangye yesterday and we think for now the user experience might be more important than software logic elegancy. Let's reconsider merging this for the coming environment release.

dachengx commented 1 year ago

@dachengx . I discussed with @jingqiangye yesterday and we think for now the user experience might be more important than software logic elegancy. Let's reconsider merging this for the coming environment release.

No objection. Just keep in mind that this will anyway take effect after the new release of nT base_environment.

FaroutYLq commented 1 year ago

Yes I am aware. Thanks for the head up

Lanqing Yuan

On Jun 10, 2023, at 1:24 PM, Dacheng Xu @.***> wrote:



@dachengxhttps://github.com/dachengx . I discussed with @jingqiangyehttps://github.com/jingqiangye yesterday and we think for now the user experience might be more important than software logic elegancy. Let's reconsider merging this for the coming environment release.

No objection. Just keep in mind that this will anyway take effect after the new release of nT base_environment.

— Reply to this email directly, view it on GitHubhttps://github.com/XENONnT/straxen/pull/1182#issuecomment-1585761765, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ALG57AXIFC3OTBQSTK377LLXKS3VPANCNFSM6AAAAAAYCYY4PE. You are receiving this because you modified the open/close state.Message ID: @.***>