(Closes #3259) shorten generated names for basis/diff-basis arrays #3264
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3264 +/- ##
=======================================
Coverage 99.91% 99.91%
=======================================
Files 376 376
Lines 53529 53553 +24
=======================================
+ Hits 53484 53508 +24
Misses 45 45 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This PR shortens the generated names used for various variables in order to avoid hitting compiler limitations as reported by Tom in #3259. Since we're no longer basing the names on existing variable names (which must be unique) I've had to increase the use of tags to ensure we cope in the face of name collisions. |
| self._boundary_dofs.append(self.BoundaryDofs(op_arg, bc_fs)) | ||
|
|
||
| def invoke_declarations(self): | ||
| def invoke_declarations(self) -> None: |
There was a problem hiding this comment.
Maybe this is a stylistic choice, but is the "-> None" needed when there is no return? None of my tooling complains with or without
| symbol = self.symtab.find_or_create_tag( | ||
| basis, | ||
| symbol_type=DataSymbol, datatype=UnsupportedFortranType( | ||
| f"real(kind=r_def), allocatable :: {basis}{dims}" |
There was a problem hiding this comment.
As you did somewhere else, you will need to do:
new_name = self.symtab.next_available_name(basis)
if you embed the name in the datatype (as it may end up being different)
| kind = const.QUADRATURE_TYPE_MAP["gh_quadrature_xyoz"]["kind"] | ||
| for name in self.qr_weight_vars["xyoz"]: | ||
| self.symtab.find_or_create( | ||
| self.symtab.find_or_create_tag( |
There was a problem hiding this comment.
new_name = self.symtab.next_available_name(...)
| :returns: a shortened form of the name. | ||
| ''' | ||
| new_parts = [] |
There was a problem hiding this comment.
Maybe its a pain to do not that you already changed lots of tests, but some of the generated code is more cryptic now, have you considered doing:
if len(name) < threshold:
return name
to keep already short names descriptive?
|
|
||
| # Split name string to find any_*_space ID and create a short name as | ||
| # "<start>" + "spc" + "ID" | ||
| # "<start>" + "s" + "ID" |
There was a problem hiding this comment.
Is this function still necessary now?, we could do:
self._shorten_arg_name(self.orig_name + "_" + arg.name)}
But if you decide not to remove the short_name do we really need to keep the _short_name and _mangled_name as state, it may be safer to compute them in the property method.
| f"space '{self._orig_name}'") | ||
|
|
||
| @staticmethod | ||
| def _shorten_arg_name(name: str) -> str: |
There was a problem hiding this comment.
Maybe just "_shorten_name", it doesn't use the fact is an arg.
No description provided.