Enable Anvil Infiniband interface #444
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #444 +/- ##
==========================================
+ Coverage 61.80% 61.99% +0.19%
==========================================
Files 62 62
Lines 7134 7133 -1
==========================================
+ Hits 4409 4422 +13
+ Misses 2725 2711 -14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/radical/utils/host.py
Outdated
| @@ -56,7 +56,7 @@ def get_hostip(req=None, log=None): | |||
|
|
|||
| white_list = [ | |||
| 'ens1f1', # amarel | |||
| # 'ib0', # infiniband | |||
| 'ib0', # infiniband | |||
There was a problem hiding this comment.
please add comment here that it is for Anvil, and that it doesn't fit Amarel (based on comments for previous commits here). Eventially these interfaces will be moved into resource configuration files
|
I added that comment - but indeed, that might break Amarel. I opened #445 for a cleaner solution. Either way, I leave it to your judgement is this should be merged (i.e., what machine you want to break :-P ) |
|
Now also fixes #445 |
| req = as_list(req) | ||
|
|
||
| white_list = [ | ||
| 'ens1f1', # amarel | ||
| # 'ib0', # infiniband | ||
| 'ib0', # infiniband # NOTE: unusable on Amarel, use `req` there! |
There was a problem hiding this comment.
+ add comment that it is for anvil
There was a problem hiding this comment.
and also, for amarel the first one ('ens1f1') should be picked, so not sure that req is require for amarel
There was a problem hiding this comment.
Oh, then I misunderstood your request above. Why was ib0 disabled then? But either way, I think that comment should better go to the amarel / anvil resource configs, and RU should stay clear of machine specific settings as much as possible. Would that work for you?
There was a problem hiding this comment.
keeping comments here before adding corresponding parameters (iface) within resource configurations
|
@andre-merzky I've added comments above |
andre-merzky
changed the title
|
@andre-merzky @mtitov, seeing that this PR is required by our IMPRESS runs on Anvil, should we consider this done and ready to be merged? |
|
TODO AM: remove default config entry for |
There was a problem hiding this comment.
LGTM!
p.s., Andre convinced me regarding this approach - DefaultConfig is a Singleton, thus its instance is created once, and it allows to share it between layers (RP and RU) - creating an instance within RP layer with the corresponding iface, then will be available within RU at get_hostip method
|
@andre-merzky ping |
addresses #443