-
Notifications
You must be signed in to change notification settings - Fork 41
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
Add sectorscalartype
#146
Add sectorscalartype
#146
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #146 +/- ##
==========================================
- Coverage 79.97% 79.95% -0.02%
==========================================
Files 42 42
Lines 4983 4974 -9
==========================================
- Hits 3985 3977 -8
+ Misses 998 997 -1 ☔ View full report in Codecov by Sentry. |
This is a good approach to eliminate all my |
As discussed, I now replaced everything with just a unified If tests turn green, this is good to go for me. |
I am wondering to what extent it would be an abomination to replace |
I have to admit that this also crossed my mind, however I was a bit worried about something like a vector of sectors which suddenly has scalartype Float64, and then the default definition of zerovector might return a vector of floats (although I did not test if this is actually the case). |
Yes you are right that this will cause unforeseen problems further on. By lack of a better suggestion I will merge as is. |
This is a minor change that centralizes the determination of the
eltype
of the topological data.This simplifies some of the fusion tree manipulations, as it makes it easier to determine the output type.
Additionally, this also provides a hook for types that don't necessarily want to have
Fsymbol(one(I), one(I),....)
, either because it is not efficient, or becauseone(I)
is hard to determine (multi-fusion stuff).Finally, it could also provide a starting point for having sectors with different eltypes, as one could imagine wanting SU2 symbols with BigFloat precision etc.
I tried to not change too much, but left a couple comments in the code about things I am unsure about. We should probably also discuss the fusion tree manipulations constructions, as the
@isdefined
construction might not be necessary now. I don't know if this has performance implications (either on the compiler or at runtime), but it might just be slightly more readible as well.