Move input files from gio #8162
Conversation
This more accurately reflects the nature of the type * Note that most of the "outputfiles" need to be readable as well because of the test harnesses * A future direction might be to make a "OutputFile" only for those cases that don't need to re-read the written data
* move to iostream via new InputFile * Rename existing OutputFiles to IOFiles
|
I just noticed that there is a failure on Windows that needs to be addressed. |
Myoldmopar
added
the
Refactoring
|
One file showing diffs is GPU, that is unrelated to this. Custom check had a warning due to a missing label, so I added refactoring. This is otherwise clean according to CI. There still aren't any conflicts, which is great. I'm quite excited about this change, what is your status? It is still marked draft, so I don't want to spend too much time digging in until it's ready. |
|
I was just waiting to make sure the CI came back clean because of the Windows fix I made. I ran cursory tests manually but didn't want to wait to run full Linux and Windows tests on my system. From my perspective, now that it looks good on the CI, it's good to go. |
|
@lefticus this branch got wildly conflicted with another branch just that merged (mine). I'm currently resolving them in this branch and will push later today. I'll let you know if I have any issues, but assuming I don't, and CI is happy upon my commit, then I think this is likely to drop soon. |
|
Now the 10.13 Macbook decided to not report. At least 10.15 is reliable. 🤷 CI is super happy here. I'm going to look it over but I already built and ran tests locally and things were good there. |
There was a problem hiding this comment.
No major concerns here. Ready for this to drop in. Thanks @lefticus!
| OutputFiles.cc | ||
| OutputFiles.hh | ||
| IOFiles.cc | ||
| IOFiles.hh |
There was a problem hiding this comment.
Indent fixes here would be good if anything else needs changing, otherwise it is fine.
| @@ -16767,7 +16771,7 @@ namespace DXCoils { | |||
| if (present(CoilIndex)) { | |||
| CoilNum = CoilIndex; | |||
| } else { | |||
| GetDXCoilIndex(state, CoilName, CoilNum, ErrorsFound); | |||
| GetDXCoilIndex(state, CoilName, CoilNum, ErrorsFound, ObjexxFCL::Optional_string_const(), ObjexxFCL::Optional_bool_const()); | |||
There was a problem hiding this comment.
I noticed a lot of places where you passed in explicit values where it was previously relying on the default _ variable. Any particular reason? Performance?
| @@ -147,11 +145,13 @@ namespace EnergyPlus { | |||
| ZoneTempPredictorCorrectorData dataZoneTempPredictorCorrector; | |||
|
|
|||
| EnergyPlusData() { | |||
| OutputFiles::setSingleton(&outputFiles); | |||
| // todo, try to eliminate the need for the singleton | |||
There was a problem hiding this comment.
This is todo for a later branch, I presume.
|
|
||
| namespace EnergyPlus { | ||
|
|
||
| class InputFile |
| @@ -197,12 +189,7 @@ set(src | |||
| src/ObjexxFCL/SetWrapper.hh | |||
| src/ObjexxFCL/Sticky.fwd.hh | |||
| src/ObjexxFCL/Sticky.hh | |||
| src/ObjexxFCL/Stream.cc | |||
Pull request overview
gio::statandgio::opencallsgiofilesNote: I changed the name of a parameter from
outputFilestofilesand this has caused a lot of conflicts in overlapping API's with @brianlball 's changes.At the moment there are no conflicts. If one arises during PR review I'm happy to correct it.