Skip to content

Conversation

@lifflander
Copy link
Contributor

Closes #16

@lifflander lifflander requested review from Copilot and nlslatt December 5, 2025 03:03
@lifflander lifflander linked an issue Dec 5, 2025 that may be closed by this pull request
@lifflander lifflander changed the base branch from master to 19-implement-basic-ci December 5, 2025 03:03
@lifflander
Copy link
Contributor Author

More test need to be written

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds unit tests for the work model functionality in the load balancing library and introduces a simplified Task constructor to facilitate test creation.

  • Adds comprehensive unit tests for WorkModelCalculator functionality including work computation, memory usage, and incremental updates
  • Introduces a simplified Task(TaskType id, LoadType load) constructor for easier task creation in tests

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/unit/work_model/test_work_model.cc New test file with 4 test cases covering work computation, memory usage calculation, memory fit checking, and work update operations
src/vt-lb/model/Task.h Adds simplified two-parameter constructor for creating Task instances with just ID and load

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

};

struct Task {
Task() = default;
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider adding documentation for this constructor to clarify its intended use case and the default values that will be used for uninitialized fields (home_, current_, migratable_, memory_, shared_blocks_). This would help other developers understand when to use this simplified constructor versus the full constructor.

Suggested change
Task() = default;
Task() = default;
/**
* @brief Simplified constructor for Task.
*
* Initializes a Task with the given id and load.
* Other fields are set to their default values:
* - home_ = invalid_node
* - current_ = invalid_node
* - migratable_ = true
* - memory_ = default-constructed TaskMemory (all fields 0.0)
* - shared_blocks_ = empty set
*
* Use this constructor when only id and load are known, and other fields can be defaulted.
* For full control, use the more detailed constructor.
*/

Copilot uses AI. Check for mistakes.
@lifflander lifflander force-pushed the 16-write-work-model-tests branch from a4c6eac to e2cee9f Compare December 5, 2025 19:21
@lifflander lifflander changed the base branch from 19-implement-basic-ci to master December 5, 2025 19:21
@lifflander lifflander force-pushed the 16-write-work-model-tests branch from e2cee9f to 822e065 Compare December 11, 2025 23:23
@nlslatt nlslatt force-pushed the 16-write-work-model-tests branch from c841482 to 8f9d587 Compare December 12, 2025 20:34
@nlslatt nlslatt force-pushed the 16-write-work-model-tests branch from 4698fc3 to f7b46c5 Compare December 15, 2025 19:56
@nlslatt nlslatt force-pushed the 16-write-work-model-tests branch from 3684e85 to 50e851e Compare December 15, 2025 22:04
@nlslatt nlslatt force-pushed the 16-write-work-model-tests branch from b946c0a to 2e4c643 Compare December 15, 2025 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Write work model tests

3 participants