search

radical-cybertools / radical.analytics

Analytics for RADICAL-Cybertools
Other
1 stars 1 forks source link

add new resource util plot type #135

Closed andre-merzky closed 3 years ago

andre-merzky commented 3 years ago

Hey @mturilli : this is the code for the new utilization plot. It depends the RP PR radical-cybertools/radical.pilot/pull/2301. Happy to iterate over it obviously, let me know what you think!

mturilli commented 3 years ago

Hi @andre-merzky , thanks that looks great. I reviewed the related RP PR and looked at this one. As we agreed before, I think it would be important to move away from a 'script' approach to RA. That is not the way most of our users need to use it and I feel we should not push for it further. I would break down the script of the PR into: a set of helper functions, some dedicated data preparation and plotting. I would then create a src/radical/analytics/utils and put the helper functions there. In this way, most of the code we now put into scripts will be put into functions, independent on how one likes to do plotting. Would that work for you? If so, happy to do the needful.

andre-merzky commented 3 years ago

Yep, I agree with that!

Hi @andre-merzky , thanks that looks great. I reviewed the related RP PR and looked at this one. As we agreed before, I think it would be important to move away from a 'script' approach to RA. That is not the way most of our users need to use it and I feel we should not push for it further. I would break down the script of the PR into: a set of helper functions, some dedicated data preparation and plotting. I would then create a src/radical/analytics/utils and put the helper functions there. In this way, most of the code we now put into scripts will be put into functions, independent on how one likes to do plotting. Would that work for you? If so, happy to do the needful.

Yup, agree with all! Feel free to take a stab at that, but also feel free to tell me to do that! :-)

andre-merzky commented 3 years ago

This is ready for another round of review - thanks.

PS.: I'll take care of the test failure