-
Notifications
You must be signed in to change notification settings - Fork 89
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
Removing ALBANY_EPETRA_EXE ifdef around distributed response / parameters code #33
Comments
The ALBANY_EPETRA ifedefs guard are mainly used to guard Epetra code. If you see parts that are guarded but do not include Epetra code than they can probably be removed. I see this as part of the refactoring to Tpetra, not sure why you see this as a separate issue. |
This issue was motivated by some confusion on my part regarding the code regarding something specific with the distributed parameters code -- maybe you can explain this to me. Are the specializations templated on PHAL::AlbanyTraits::DistParamDeriv, Traits meant to work when ALBANY_EPETRA_EXE is off? If you look at the code, some of these specializations have ALBANY_EPETRA_EXE guards (the ones that use Epetra objects from the workset, as expected), and some don't. I am wondering if it is safe for me to remove ALBANY_EPETRA_EXE ifdef guards around new code I checked in today that is Epetra-free, or if this is going to cause issues in a no-Epetra build. It seems to me like there should be issues when using distributed parameters when Epetra is turned off b/c, for example, there are ifdef guards in critical functions in PHAL_SeparableScatterScalarResponse_Def.hpp; yet my tests that use distributed parameters run correctly even when Epetra is off. |
I don't know. I guess we should try and see.
There might even be some code that is not used any longer, or maybe in the Tpetra version there is some missing logic so that not all the parts of the sensitivities are computed (but that should not be caused by ifdef guards). I guess it is probably better to first figure out what's wrong with the ATO tests and then we can try to clean the #ifdef guards. |
I see. This is along the lines of what I was conjecturing. I'll keep this open for a bit until I understand better the behavior with ATO. Then I agree this can be closed and moved to the infrastructure cleanup task/issue. |
Closing this issue - it will be moved to general Albany infrastructure cleanup issue / task. |
If distributed responses / parameters are meant to work without Epetra in Albany, the ALBANY_EPETRA_EXE ifdef guards around the relevant functions should be removed. I am concerned if this is not done, there may be issues for physics that are being ported to Tpetra and that utilize this capability (e.g., ATO).
The text was updated successfully, but these errors were encountered: