-
Notifications
You must be signed in to change notification settings - Fork 23
Update generated code for DPF 252_daily on master #2016
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
Conversation
| server=None, | ||
| ): | ||
| super().__init__(name="mapdl::rth::TF_cyclic", config=config, server=server) | ||
| super().__init__(name="mapdl::rst::TF_cyclic", config=config, server=server) |
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.
@oparreno Just confirmation, as I can see that this was merged yesterday: Was this a bug before? Should it be rst or rth? Thanks!
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.
@rafacanton, @ricardopeixotocoelho created a PR to simplify the reader operators from RST and make previous calls to RTHSource go to Elemental and Nodal source from RST. I assume that this change just changed the operator name? Both should be supported as TF could be extracted from RST & RTH as it is a thermal result.
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.
@rafacanton @oparreno @ricardopeixotocoelho so this here means that the latest recorded public operator with scripting_name=cyclic_expanded_heat_flux is the RST one. It makes sense since you switched it from RTH to RST.
The PR being worked on server-side adds back the record for RTH. The thing is that if both have the same scripting name, then one will override the other and you will end-up with a name here of format mapdl::xxx::TF_cyclic.
What you can do is make those private, and record a public TF_cyclic operator which redirects based on the active namespace for the result. @rafacanton could you maybe explain how to do that properly?
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 think the PR to be merged server-side fixes this
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2016 +/- ##
==========================================
+ Coverage 87.47% 88.61% +1.14%
==========================================
Files 90 90
Lines 10254 10254
==========================================
+ Hits 8970 9087 +117
+ Misses 1284 1167 -117 |
a3cd0af to
c940dd7
Compare
c940dd7 to
26803a4
Compare
| _globals['_GETASRESPONSE']._serialized_start=648 | ||
| _globals['_GETASRESPONSE']._serialized_end=1529 | ||
| _globals['_CREATEREQUEST']._serialized_start=1532 | ||
| _globals['_CREATEREQUEST']._serialized_end=1716 |
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.
@cbellot000 @rafacanton @ansys-akarcher is that retrocompatible?
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.
@PProfizi @ansys-akarcher @cbellot000 I think this is due to the new API added to the Any for FCs, and this is a way to parse the updated string above.
|
|
||
|
|
||
| class expansion_psd(Operator): | ||
| """Computes the PSD response for one-sigma solution. |
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.
Hi @oparreno could we maybe have a more extensive description of the goal of the operator here?
| Parameters | ||
| ---------- | ||
| solver_coordinate_system_ids : int, optional | ||
| Coorfinate system ids to recover used by the |
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.
| Coorfinate system ids to recover used by the | |
| Coordinate system ids to recover used by the |
@oparreno can we really ask for several? The typehint says it expects a single int.
|
Merging as this is blocking development. Still, the different issues raised should be treated. |
An update of generated code has been triggered either manually or by an update in the dpf-standalone repository.