-
Notifications
You must be signed in to change notification settings - Fork 34
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
Modules + free form + fpm + cmake #70
base: main
Are you sure you want to change the base?
Conversation
src/libsqrt.f90
Outdated
II_U = (I_U**2 - I_T)/2. | ||
|
||
sqrt_2 = 1./(I_U*II_U - III_U) & | ||
*(I_U*III_U*identity2(T) + (I_U**2 - II_U)*T - T.pow. (2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adtzlr can you just sanity check this for me? this used to be T2 and I think because of how interfaces were created it didn't die, but I see this as tensor^2 not Tensor2 based on the overloaded ** operator
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @JorgeG94 for pointing that out! The original implementation gives the correct result, because the overloaded **
(double-dot) operator is for Tensor**Tensor
operations only. So Tensor**Integer
refers to the Matrix product, which is correct in this formula. No need for explicit .pow.
here. I know, this can be confusing. Once you get the logic, it's very similar to hand-written matrix notation. I'd like to keep this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was having some issues with derived types since they are not going through explicit interfaces I believe, if I went back to ** it would not compile ! I will investigate this though.
Thanks @JorgeG94 for all the proposed changes. Please give me some time to go through all the changes (and I'll definitely come back with some questions...). It will take me some time ⏳ but your effort is very welcome 🚀! This was more or less the kind of contribution I was looking for because it is way out of my scope. |
Thanks so much! I had quite a bit of fun doing this. Although this are breaking changes because of how the project can be included in other projects it is certainly the best way to go for the future. Modules + explicit interfaces are a great thing to have, they prevent many errors and bugs by doing checks at compile time. Adding build systems that suit everyone's needs is also cool I am not sure how the commercial packages you mention are built inhouse, but surely this cannot hurt. A potential cool thing to do would be to have a general way of handling tensors, I see that they are all set to be Cartesian i.e. Tensor1 is of size 3, etc. |
Definitely, like |
This PR will introduce breaking changes if the apps that use it are not updated accordingly!
A couple of things that still need to be done:
.pow.
is correct