Skip to content

Add auto_num_nodes smart attribute - #5699

Open
bstepanovski wants to merge 1 commit into
masterfrom
feature/auto-num-nodes-5680
Open

bstepanovski wants to merge 1 commit into
masterfrom
feature/auto-num-nodes-5680

Conversation

@bstepanovski

Copy link
Copy Markdown
Contributor

Fixes #5680

Adds an auto_num_nodes smart attribute for dynamically driven node counts.

  • Adds auto_num_nodes as a number field with the same scheduler submit behavior as bc_num_nodes
  • Keeps bc_num_nodes for static limits so implementers can distinguish dynamic/fixed fields
  • Wires Project Manager support
  • Covers defaults and submit formats with a unit test, and includes it in the PM system test

@Bubballoo3

Copy link
Copy Markdown
Contributor

IIRC this will depend upon OSC/ood_core#955 being implemented first, otherwise auto_queues won't have enough info to set the min and max dynamically as this ticket suggests. It's a bit confusing though because auto_cores should also be dynamically set but is not currently (#5677). But aside from that essential behavior being missing, I think we will likely want to just inherit the submission info from bc_num_nodes.

Philosophically, this should be a 'version of bc_num_nodes which has its min and max set by other auto_ fields', so keeping a single source of truth for the appearance and submission details for each will make sense. However implementing it before the min and max can be set is a bit useless since then it is just another bc_num_nodes

end

module Attributes
class AutoNumNodes < Attribute

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You mention bc_num_nodes in a comment, shouldn't we just subclass here?

@johrstrom

Copy link
Copy Markdown
Contributor

However implementing it before the min and max can be set is a bit useless since then it is just another bc_num_nodes

Not sure if I follow what you're saying exactly. I mean I get it, but I can't tell if it's a call to action (like to hold this PR) or purely informational.

@Bubballoo3

Copy link
Copy Markdown
Contributor

I can't tell if it's a call to action (like to hold this PR) or purely informational.

Definitely a call to hold the PR, as it seems risky to merge it in an incomplete form like this. I can add an 'on hold' label right now

@Bubballoo3 Bubballoo3 moved this from Awaiting Review to On Hold in PR Review Pipeline Aug 20, 2026
@bstepanovski

bstepanovski commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

That makes sense. Happy to hold this until OSC/ood_core#955 gets implemented. I can pick it back up then

@johrstrom

Copy link
Copy Markdown
Contributor

I can't tell if it's a call to action (like to hold this PR) or purely informational.

Definitely a call to hold the PR, as it seems risky to merge it in an incomplete form like this. I can add an 'on hold' label right now

Not sure why. When the ood_core update happens, all the changes will be on auto_queues to set data attributes. Maybe some additional tests against this, but this class and view(s) won't change. Or am I missing something?

@johrstrom

Copy link
Copy Markdown
Contributor

Thinking about this more, I think this PR should just add the smart attribute and a test. The PM changes look fine in and of themselves, but I feel like we should replace bc_num_nodes instead of adding a new field, but that may take some consideration on how to upgrade forms/launchers safely.

@bstepanovski bstepanovski changed the title Add auto_num_nodes smart attribute (#5680) Add auto_num_nodes smart attribute Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: On Hold

Development

Successfully merging this pull request may close these issues.

auto_num_nodes

4 participants