Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

Clean up N_MAX objects #228

Merged
merged 6 commits into from
Jan 4, 2023
Merged

Conversation

YonsiG
Copy link
Contributor

@YonsiG YonsiG commented Jan 2, 2023

this is a clean up PR following #222 , removing redundant mess up of duplicate commits. Double check with nvprof and cuda-memcheck, see no errors.

SDL/Event.cu Outdated
@@ -5,6 +5,7 @@ struct SDL::modules* SDL::modulesInGPU = nullptr;
struct SDL::pixelMap* SDL::pixelMapping = nullptr;
uint16_t SDL::nModules;
uint16_t SDL::nLowerModules;
unsigned int nTotalSegments;
Copy link
Contributor

Choose a reason for hiding this comment

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

I can now see that you don't duplicate the creation of segment arrays, taking into account my comment (thank you, I will resolve that discussion). Because of this, I understand that you had to define the nTotalSegments in a more global scope, so that you can use it in multiple functions within the Event class.

However, you define it as a global variable and this can change from event to event. If we run multi-streaming, I would expect it to be changed by all of the events that run concurrently, leading to unexpected behavior, since all of them will have different values for that same variable. To save ourselves from this situation, I would make the nTotalSegments variable a private variable of the Event class (like this one), so that each event can have its own value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for pointing it out Manos! I've put this into Event class to correct this :)

SDL/Event.cu Outdated
createSegmentArrayRanges(*modulesInGPU, *rangesInGPU, *mdsInGPU, nLowerModules, nTotalSegments, stream, N_MAX_SEGMENTS_PER_MODULE, N_MAX_PIXEL_SEGMENTS_PER_MODULE);
// cout<<"nTotalSegments: "<<nTotalSegments<<std::endl; // for memory usage

//problem here: didn't distinguish pixel segments and outtracker segments. so they use the same memory index, which should be different and allocate dynamically
Copy link
Contributor

Choose a reason for hiding this comment

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

Please, check the discussion on the same line from the previous PR.

@bsathian
Copy link
Contributor

bsathian commented Jan 4, 2023

Looks good. I'm merging this

@bsathian bsathian merged commit e297fb5 into SegmentLinking:master Jan 4, 2023
@VourMa VourMa linked an issue Mar 20, 2023 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is printMDs broken?
3 participants