Addressbook #121
Addressbook #121
Conversation
…overing bad link/map from integrity check
…biguation of a Union
mvousden
left a comment
mvousden
left a comment
There was a problem hiding this comment.
Points:
-
I like your documentation. I would add something about the notion of integrity to your documentation, since those checks are the most complicated section of this changeset, and it took me a while to figure out what was being checked when (and by what, in the case of the multithreaded implementation).
-
Code structure is consistent, which made it easy for me to find methods and names I was looking for.
-
I would welcome docstrings (in the source) for the methods that do not have them, though I grant you it is somewhat intuitive what the methods do at points.
-
Your documentation claims that
int AddressBook::TaskCountexists when it doesn't. I agree that it shouldn' existt (you've got a method that simply queries the size ofAddressBook::TaskMap, which is fine), but I wonder if there are other discrepancies I have not found. -
Typo in
AddressBookDocs.pdf: "Task Data structs (TaskData_t) represent a task without and of the fast access maps and vectors." -
Could you put your documentation on the shared drive and commit it to the orchestrator-documentation repository when done?
Happy Mark, no 🍠 for you.
| unsigned long Rank; // OR Supervisor's MPI Rank. | ||
| }; | ||
| DTypeIdx DeviceType; // Index of the Device's type in task. | ||
| AttrIdx Attribute; // Index of the Device's attribute in task. |
There was a problem hiding this comment.
I don't understand this comment - what is a "Device's attribute"?
There was a problem hiding this comment.
A device can supposedly have a free-form attribute that can be stored - this facilitates that
There was a problem hiding this comment.
What does that mean in the wider context of POETS?
This PR splits the AddressBook from the Nameserver that was submitted as part of #103. Comments made during that PR are addressed, a series of tests (more to be added later) in the current testing framework are added and the AddressBook sources have been moved to a sub-directory of Nameserver.
This is not currently used by the Orchestrator, but is built and tested with
make tests.Two bits of preliminary documentation included:
AddressBookDocs.pdf
AddrBook.pdf