Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merlin/bridge: load user subcomponent for networkIF #2407

Conversation

heyitsanthony
Copy link
Contributor

pymerlin expects to override a SimpleNetwork subcomponent with its own network subcomponent (e.g., LinkControl). When using a Bridge, this subcomponent is ignored and link initialization crashes on SST::Link::sendUntimedData when trying to access a null rtr object. This patch extends the Bridge component to use the user subcomponent if set and if not then fall back to using an anonymous subcomponent as before.

@sst-autotester
Copy link
Contributor

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

Copy link
Contributor

@feldergast feldergast left a comment

Choose a reason for hiding this comment

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

Please change things to use the SubComponentSlotInfo interface for creating the user subcomponents.

nic.nic = loadAnonymousSubComponent<SST::Interfaces::SimpleNetwork>
("merlin.linkcontrol", "networkIF", id,
ComponentInfo::SHARE_PORTS | ComponentInfo::INSERT_STATS, if_params, 1 /* vns */);
nic.nic = loadUserSubComponent<SST::Interfaces::SimpleNetwork>
Copy link
Contributor

Choose a reason for hiding this comment

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

This should really use one slot name (networkIF), with multiple instances using an index. In the c++ code, this would use the SubComponentSlotInfo object to load the user subcomponents. The definition of the class is in baseComponent.h and you would get if by calling:

getSubComponentSlotInfo("networkIF")

This object allows you to query which indices have been loaded with a user subcomponent.

The code would look something like:

    SubComponentSlotInfo* info = getSubComponentSlotInfo("networkIF");
    if ( !info ) {
        // Create anonymous subcomponent
    }
    info->create<SimpleNetwork>(id, ComponentInfo::SHARE_PORTS, 1)

There are also other functions that could be used to do more complete error checking to see if the right number of subcomponents had been added, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, thank you for the fast response and detailed explanation on how to properly support this. I updated the code to behave as follows:

  • load anonymous subcomponent if no info
  • load anonymous component if id is not populated (for half-anonymous case)
  • load user component if id is populated
  • fatal out if more than two slots are given

I was passing "networkIF1" to NetworkInterface._instanceNetworkInterfaceBackCompat beforehand; I changed it to "networkIF" with slot 1 and it works great!

pymerlin expects to override a SimpleNetwork subcomponent with its own
network subcomponent (e.g., LinkControl). When using a Bridge, this
subcomponent is ignored and link initialization crashes on
SST::Link::sendUntimedData when trying to access a null rtr object. This
patch extends the Bridge component to use the user subcomponent if the
subcomponent slot is populated and if not then fall back to using an
anonymous subcomponent as before.
@sst-autotester
Copy link
Contributor

Status Flag 'Pre-Test Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
NO INSPECTION HAS BEEN PERFORMED ON THIS PULL REQUEST! - This PR must be inspected by setting label 'AT: PRE-TEST INSPECTED'.

@sst-autotester
Copy link
Contributor

Status Flag 'Pre-Test Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED by label AT: PRE-TEST INSPECTED! Autotester is Removing Label; this inspection will remain valid until a new commit to source branch is performed.

@sst-autotester
Copy link
Contributor

Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects:

Pull Request Auto Testing STARTING (click to expand)

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements

  • Build Num: 1665
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_Make-Dist

  • Build Num: 1071
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MT-2

  • Build Num: 1623
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MR-2

  • Build Num: 1623
  • Status: STARTED

Build Information

Test Name: SST__AutotestGen2_NewFW_OSX-14-XC15-ARM2_OMPI-4.1.6_PY3.10_sst-elements

  • Build Num: 195
  • Status: STARTED

Using Repos:

Repo: ELEMENTS (heyitsanthony/sst-elements)
  • Branch: anthony/bridge-user-networkIF
  • SHA: c273614
  • Mode: TEST_REPO
Repo: SQE (sstsimulator/sst-sqe)
  • Branch: devel
  • SHA: 2574c98896598820227190149834172b962dc3fc
  • Mode: SUPPORT_REPO
Repo: CORE (sstsimulator/sst-core)
  • Branch: devel
  • SHA: 335de84417671898414e7f4d242c5ab109d5f963
  • Mode: SUPPORT_REPO
Repo: MACRO (sstsimulator/sst-macro)
  • Branch: devel
  • SHA: 50a62170b3681ea20cc2f56abd2eb3911053f1fc
  • Mode: SUPPORT_REPO

Pull Request Author: heyitsanthony

@sst-autotester
Copy link
Contributor

Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED

Pull Request Auto Testing has PASSED (click to expand)

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements

  • Build Num: 1665
  • Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_Make-Dist

  • Build Num: 1071
  • Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MT-2

  • Build Num: 1623
  • Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_sst-test_OMPI-4.1.4_PY3.6_sst-elements_MR-2

  • Build Num: 1623
  • Status: PASSED

Build Information

Test Name: SST__AutotestGen2_NewFW_OSX-14-XC15-ARM2_OMPI-4.1.6_PY3.10_sst-elements

  • Build Num: 195
  • Status: PASSED

@sst-autotester
Copy link
Contributor

Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging
THE LAST COMMIT TO THIS PULL REQUEST HAS NOT BEEN REVIEWED YET!

@sst-autotester
Copy link
Contributor

All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur...

@hughes-c hughes-c added this to the SST v15.0.0 milestone Oct 8, 2024
@sst-autotester
Copy link
Contributor

All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur...

@sst-autotester
Copy link
Contributor

Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ feldergast ]!

@sst-autotester
Copy link
Contributor

Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged

@sst-autotester sst-autotester merged commit 9a211a9 into sstsimulator:devel Oct 8, 2024
3 checks passed
@sst-autotester
Copy link
Contributor

Merge on Pull Request# 2407: IS A SUCCESS - Pull Request successfully merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants