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

Epetra: throw error instead of segfault when using more than MAX_INT local indices #10249

Closed
jpthiele opened this issue Feb 25, 2022 · 9 comments
Assignees
Labels
type: enhancement Issue is an enhancement, not a bug

Comments

@jpthiele
Copy link

jpthiele commented Feb 25, 2022

Enhancement

@trilinos/epetra

When using more than MAX_INT local entries in Epetra_CrsGraph the code runs into a segfault
when calling OptimizeStorage() as this results in negative local indices.

There are two places in MakeLocalIndices 1 and 2
in which the resulting local index is checked for an error in Epetra_BlockMap::LID(gid)
by comparing the result with -1.
Couldn't (result < 0) be checked instead to catch this error as well?

@jpthiele jpthiele added the type: enhancement Issue is an enhancement, not a bug label Feb 25, 2022
@jpthiele jpthiele changed the title PackageName: General Summary of the Enhancement Epetra: throw error instead of segfault when using more than MAX_INT local indices Feb 25, 2022
@bangerth
Copy link
Contributor

This is part of the explorations in dealii/dealii#13428 where we're tracking down what happens if one tries to allocate sparsity patterns with more than 2B entries on one process :-(

@github-actions
Copy link

This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity.
If you would like to keep this issue open please add a comment and/or remove the MARKED_FOR_CLOSURE label.
If this issue should be kept open even with no activity beyond the time limits you can add the label DO_NOT_AUTOCLOSE.
If it is ok for this issue to be closed, feel free to go ahead and close it. Please do not add any comments or change any labels or otherwise touch this issue unless your intention is to reset the inactivity counter for an additional year.

@github-actions github-actions bot added the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Feb 26, 2023
@jhux2
Copy link
Member

jhux2 commented Feb 27, 2023

@ccober6

@ccober6
Copy link
Contributor

ccober6 commented Feb 27, 2023

@jpthiele and @bangerth, just wanted to make sure you were aware that the Epetra stack (i.e., Epetra and all its dependent Trilinos packages) is slated for archival (removed from Trilinos but have their own repos) at the end of FY25 (Sep. 2025). You should factor this into your future plans.

@github-actions github-actions bot removed the MARKED_FOR_CLOSURE Issue or PR is marked for auto-closure by the GitHub Actions bot. label Mar 1, 2023
@GrahamBenHarper GrahamBenHarper self-assigned this Mar 13, 2023
@GrahamBenHarper
Copy link
Contributor

Hi @jpthiele, sorry for the delay in action. This seems like a reasonable fix. Have you personally tested this? If not, I can look into this and submit a PR.

@jpthiele
Copy link
Author

@GrahamBenHarper I only briefly looked into it quite a while ago, so it would be perfect if you can look into it.

@ccober6 I knew that it is out of active development in terms of new features, but I did not know of the specific date yet. That is certainly good to know. At the time this issue seemed like an easy-ish fix that could nicely be cherry-picked even when installing an older version of Trilinos.

I also know that some people looked into writing wrappers for Tpetra in deal.II but I don't know the specific reasons why certain initiatives stopped. For me personally it is currently an issue of time as multiple other things have a higher priority at the moment.

@GrahamBenHarper
Copy link
Contributor

@jpthiele will do!

@ccober6
Copy link
Contributor

ccober6 commented Mar 16, 2023

@jpthiele Yeah, I just wanted to make sure it was on your radar. We should continue to make bug fixes until we are closer to the archival date, but obviously major changes/fixes we would need to assess the cost/benefit. :)

@GrahamBenHarper
Copy link
Contributor

@jpthiele @bangerth I merged the suggested fix, so I'll close this as completed. Please reopen this if issues persist with CrsGraph, and please let me know if you run into similar issues in other parts of Epetra.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Issue is an enhancement, not a bug
Projects
None yet
Development

No branches or pull requests

5 participants