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

Communication per processor and over time functionality has been added #139

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dhruvr4
Copy link
Contributor

@dhruvr4 dhruvr4 commented Aug 28, 2021

No description provided.

@dhruvr4 dhruvr4 requested a review from rbuch as a code owner August 28, 2021 03:20
Comment on lines +153 to +154
communicationMenuItem.setEnabled(true);
communicationVsTimeMenuItem.setEnabled(true);
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 be conditioned on the logs having a certain version, otherwise this will be broken for older logs.

Comment on lines +370 to +381
worker = new SwingWorker() {
public Object doInBackground() {
return null;
}
public void done() {
getData();
setOutputGraphData();
Checkbox cb = cbg.getSelectedCheckbox();
setCheckboxData(cb);
thisWindow.setVisible(true);
thisWindow.repaint();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is almost exactly duplicated from the hasLogFiles() case, can they be combined?

Comment on lines +44 to +49
private double[] msg_count;
private double[] msg_size;
private double[] msg_recv_count;
private double[] msg_recv_size;
private double[] msg_recv_count_ext;
private double[] msg_recv_size_ext;
Copy link
Contributor

Choose a reason for hiding this comment

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

These should all be read as RLEBlocks to match the existing way of parsing SumDetail data.

@@ -165,7 +171,177 @@ protected void read()
buildTable(TOTAL_TIME);
} else if (label.equals("EPCallTimePerInterval")) {
buildTable(NUM_MSGS);
} else {
} else if (label.equals("MsgSentCount")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is all unnecessarily duplicated, it should all be using the same method with a parameter for each different type of data. As commented above, this should all be read as RLEBlocks like execution and call time already are, so ideally this should also be read in buildTable just as those are.

Comment on lines +452 to +478
public double[] getMsg_count()
{
return msg_count;
}

public double[] getMsg_size()
{
return msg_size;
}
public double[] getMsg_recv_count()
{
return msg_recv_count;
}

public double[] getMsg_recv_size()
{
return msg_recv_size;
}
public double[] getMsg_recv_count_ext()
{
return msg_recv_count_ext;
}

public double[] getMsg_recv_size_ext()
{
return msg_recv_size_ext;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, as stated earlier, these can use the existing getData method if they are read in as RLEBlocks.

Comment on lines +259 to +285
public double[][] getMsg_count()
{
return msg_count;
}

public double[][] getMsg_size()
{
return msg_size;
}
public double[][] getMsg_recv_count()
{
return msg_recv_count;
}

public double[][] getMsg_recv_size()
{
return msg_recv_size;
}
public double[][] getMsg_recv_count_ext()
{
return msg_recv_count_ext;
}

public double[][] getMsg_recv_size_ext()
{
return msg_recv_size_ext;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can all be done via the existing getData function, see other comments.

Comment on lines +49 to +54
private double[][] msg_count;
private double[][] msg_size;
private double[][] msg_recv_count;
private double[][] msg_recv_size;
private double[][] msg_recv_count_ext;
private double[][] msg_recv_size_ext;
Copy link
Contributor

Choose a reason for hiding this comment

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

These should all be removed in favor of using rawData, see other comments.

Comment on lines +360 to +383
public double[][] getMsg_count()
{
return intervalData.getMsg_count();
}
public double[][] getMsg_size()
{
return intervalData.getMsg_size();
}
public double[][] getMsg_recv_count()
{
return intervalData.getMsg_recv_count();
}
public double[][] getMsg_recv_size()
{
return intervalData.getMsg_recv_size();
}
public double[][] getMsg_recv_count_ext()
{
return intervalData.getMsg_recv_count_ext();
}
public double[][] getMsg_recv_size_ext()
{
return intervalData.getMsg_recv_size_ext();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation and style, try to use camelCase and avoid using _ in function/variable names if possible.

for(int j =0;j<sentMsgCount_temp[i].length;j++){
int curInt= j/numEPs ;
int curEP= j-curInt*numEPs;
curInt = (curInt * 1000)/numIntervals2;
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this 1000 coming from? What exactly is this trying to do? Resample the number of intervals in the logs to be 0-1000?

@PathikritGhosh
Copy link

This will need to be changed for column major processing, instead of row major.

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.

3 participants