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

IJ assembly memory #1099

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

IJ assembly memory #1099

wants to merge 27 commits into from

Conversation

liruipeng
Copy link
Contributor

@liruipeng liruipeng commented Jun 17, 2024

This PR adds early assembly option to optimize the memory usage in the IJ interface to assemble matrices on GPUs.

User level APIs are

HYPRE_IJMatrixSetEarlyAssemble
HYPRE_SStructMatrixSetEarlyAssembly

@@ -511,6 +511,10 @@ HYPRE_SStructMatrixDestroy(HYPRE_SStructMatrix matrix);
HYPRE_Int
HYPRE_SStructMatrixInitialize(HYPRE_SStructMatrix matrix);

HYPRE_Int
HYPRE_SStructMatrixSetEarlyAssembly( HYPRE_SStructMatrix matrix,
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you use the name "Assembly" but in IJ you use "Assemble".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, they need to be consistent. Maybe both use "Assemble"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Either is fine with me, but maybe "Assemble" is better because it connects more explicitly to the assemble function.

Copy link
Contributor

@rfalgout rfalgout left a comment

Choose a reason for hiding this comment

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

This looks good to me. Thank!

@@ -344,6 +344,15 @@ HYPRE_Int HYPRE_IJMatrixSetDiagOffdSizes(HYPRE_IJMatrix matrix,
HYPRE_Int HYPRE_IJMatrixSetMaxOffProcElmts(HYPRE_IJMatrix matrix,
HYPRE_Int max_off_proc_elmts);

HYPRE_Int HYPRE_IJMatrixSetInitAllocation(HYPRE_IJMatrix matrix,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we document these functions in this PR?

hypre_IJMatrixTranslator(matrix) = aux_matrix;
}
hypre_AuxParCSRMatrixInitAllocFactor(aux_matrix) = factor;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#endif
#else
HYPRE_UNUSED_VAR(matrix);
HYPRE_UNUSED_VAR(factor);
#endif

hypre_IJMatrixTranslator(matrix) = aux_matrix;
}
hypre_AuxParCSRMatrixEarlyAssemble(aux_matrix) = early_assemble;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#endif
#else
HYPRE_UNUSED_VAR(matrix);
HYPRE_UNUSED_VAR(early_assemble);
#endif

hypre_IJMatrixTranslator(matrix) = aux_matrix;
}
hypre_AuxParCSRMatrixGrowFactor(aux_matrix) = factor;
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#endif
#else
HYPRE_UNUSED_VAR(matrix);
HYPRE_UNUSED_VAR(factor);
#endif

Copy link
Contributor

@victorapm victorapm left a comment

Choose a reason for hiding this comment

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

Thanks, Rui Peng!

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