PP-740: Add Cray platform detection in PTL

Developers
1

Hi All,

Please review the design document created for “Cray platform detection” as part of Cray PTL support.
https://pbspro.atlassian.net/wiki/display/PD/PP-740%3A+Add+Cray+platform+detection+in+PTL

Please do let me know of any comments or suggestions.

Thanks and Regards,
Sanket

2

Hey Sanket,

One question: what will “get_platform()” return when it’s an actual Cray & when it’s Cray simulator? I think it’ll be useful to specify the values in the design doc.

And do we really need all three interfaces? It seems a bit of an overkill, one can just do: if get_platform() == “Cray” instead of calling is_cray() right? So, maybe just get_platform() is enough?

Thanks,
Ravi

3

Hi @agrawalravi90,

get_platform() returns string as “cray” if it’s a actual Cray cluster and returns string as “craysim” if it’s a Cray simulator.
I have updated and mentioned these two string values in doc.

Talking about three interfaces, if test user wants to just check whether the platform is Cray or Cray simulator then he can just call “is_cray()” or “is_cray_simulator()” for any specified system in a cluster, which will return true or false. User doesn’t need to remember strings (‘cray’, ‘craysim’) and compare it, which will simplify users job in my opinion. Also in very rare case if we change these strings internally then it will affect at all places.

Thanks,
Sanket

4

Ah, ok, I see their value now. Alright, this looks good to me, I sign-off on the document, but I would request @lisa-altair to review this once.

5

By “path to cname” do you mean /proc/cray_xt/cname? I think you should be specific when you refer to the paths that will be checked.

6

Hi @borlesanket,
Do you see a need for a test writer to call “get_platform”?
If not, I think you should NOT expose “get_platform” as an interface, and only expose the 2 new interfaces that you expect test writers to use: “is_cray” and “is_cray_simulator”.
Because paraphrasing what @agrawalravi90 said these three interfaces seems like a bit of overkill.

7

Hi @lisa-altair,
I don’t see any need for a test writer to call “get_platform”, he or she can just call “is_cray” and “is_cray_simulator”. I have removed get_platform interface and updated the design doc as per requirement.

Hi @mkaro,
Yes, path to cname means “/proc/cray_xt/cname”, but as replied above to Lisa, I have removed “get_platform” interface, which was having these paths mentioned.

8

Thanks @borlesanket. Now that you have removed get_platform, it would be good to remove the references to it in the details section of the remaining interfaces (e.g. “Call DshUtils.get_platform(),”). That is implementation detail. :wink:
Other than that, it looks good.

9

I agree with @lisa-altair. Please remove the twol lines that reference get_platform.

10

Hi @lisa-altair, @mkaro Thanks, I have updated the doc as per requirement. Please have a look.

11

Hey guys,

get_platform() is an existing interface, so I think we do need to modify that as part of PP-740 to now also support Cray, so I’d add that back in.

Thanks,
Ravi

12

Looks fine to me. You have my signoff.

13

@borlesanket
The PTL has an interface get_platform() to detect platform. You can update the existing get_platform() for new platforms and it will suffice your requirements.

get_platform() API document should say what all platform it can detect. I think we don’t need new interface for each platform.

1 Like
14

Hi @kjakkali , I have made changes as per the requirement. Please have a look.

15

@borlesanket Changes looks good to me. i sign off.

16

@borlesanket Design document looks good to me. I sign-off.

17

Looks good to me as well, thanks for making the changes Sanket.

18

Hi @mkaro, @lisa-altair, I have made changes as per Kumar’s comment , Please have a look.

19

Looks good. I sign off @borlesanket.

20

Thanks for making the change @borlesanket. Looks good.