-
Notifications
You must be signed in to change notification settings - Fork 389
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
SPM - Fortran Transition - throw if SPM names are not unique #10542
Conversation
CALL GetObjectItem(SPMTypes(i),spmNum,Alphas,NumAlphas,Numbers,NumNumbers,Status) | ||
! VerifyName(TRIM(Alphas(1)), SPMNames, spmIndex, SPMErrorFound, IsBlank, "SetpointManager Unicity"); | ||
IF (FindItemInList(TRIM(Alphas(1)), SPMNames, spmIndex) /= 0) THEN | ||
CALL ShowFatalError('SetpointManager Unicity of Names: SPM of type '//TRIM(SPMTypes(i))//' has a name already found='//TRIM(Alphas(1)),Auditf) |
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.
Could you just append a loop or counter index to make the name unique and continue searching?
|
||
INTEGER, PARAMETER :: NumSPMTypes = 29 | ||
CHARACTER(len=MaxNameLength), DIMENSION(NumSPMTypes) :: SPMTypes | ||
SPMTypes(1) = "SETPOINTMANAGER:SCHEDULED" |
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 can't really read Fortran, and by "can't" I mean "don't want to." Why is this thing in Fortran and not in Python? This seems like a very Python thing.
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.
You are right that this is a Python thing. It is actively being worked on by @jasondegraw, and it looks great. We'll get there.
I forgot to add the license text I guess. Edwin just mentionned this would be needed, and I nerdsnipped myself into helping out.
Anyways, I agree this would be better in any other language, even C++. Python is a good candidate, would make it much easier to write transition rules (and we could even have - hear me out - tests!). @Myoldmopar long had a vision to integrate seemlessly python utilities in our install of E+, the issue has always been (IIUC) packaging. I think I've got a solution, "just" make it part of the CLI as a subcommand, and use the embedded python to execute the modules. And TADAAAA |
CALL GetObjectItem(SPMTypes(i),spmNum,Alphas,NumAlphas,Numbers,NumNumbers,Status) | ||
! VerifyName(TRIM(Alphas(1)), SPMNames, spmIndex, SPMErrorFound, IsBlank, "SetpointManager Unicity"); | ||
IF (FindItemInList(TRIM(Alphas(1)), SPMNames, spmIndex) /= 0) THEN | ||
CALL ShowFatalError('SetpointManager Unicity of Names: SPM of type '//TRIM(SPMTypes(i))//' has a name already found='//TRIM(Alphas(1)),Auditf) |
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.
Unicity. TIL
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.
This looks fine, and I confirmed it works by making a non-unicity-based input file:
** Fatal ** SetpointManager Unicity of Names: SPM of type SETPOINTMANAGER:MIXEDAIR has a name already found=Mixed Air Temp Manager 1
************* Conversion Terminated--Fatal Error Detected. 1 Warning; 0 Severe Errors
Late to the party here. I don't think we want transition to fatal. If transition isn't going to fix the names, then throw a warning but continue and convert the rest of the file. IMHO it makes more sense as a user to repair the newer version file rather than go back and change the original. |
So I know it says it fatalled....but it actually created the new input file properly with 24.2 version and everything. Here's the vCpErr:
I agree it shouldn't fatal. And if it's reporting a fatal and continuing anyway, it shouldn't be reporting a fatal error. We can fix it over in the SPM branch where this code now lives. (It's not in develop yet, this PR just put it into SPM) |
Pull request overview
ref #10537 (comment)
Testbed on compiler explorer: https://gcc.godbolt.org/z/KK4e64c8b
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.