File IO improvements #319

smavros · 2017-10-04T13:49:19Z

addressees #31 #193 and #312

- Modified the Parameters_Method_* and Method_* source files - Modified Logging, State, Configparser and Configwriter source files - Modified the default input file and the input files of the unit tests - Modified the INPUT documentation

GPMueller · 2017-10-04T14:36:56Z

This is also related to #166, #220 and #260.

- Added IO test - Improved OVF writer - Added IO files of unit tests to gitignore - Added gitkeep in IO folder of unit tests

- Improved IO test - Added Require_Single() method in Filter_File_Handle

smavros · 2017-10-25T14:09:52Z

- added SPIRIT in name of corresponding VF_FileFormat - extended OVF to OVF_bin8, OVF_bin4 and OVF_text - added Save_to_SPIRIT() for every SPIRIT fileformat - changed parameters to Datawriter.cpp functions for images to vectorfield and geometry - added comments to IO test - improved code formating and indentation

…vements

…-code#338

- rectangular mesh - 8byte binary - text - Split the data writing in different functions

- renamed Write_Spin_Configurations_Chain() to Write_Chain_Spin_Configuration() - added Write_SPIRIT_Version() - implemented IO_Chain_Append() - added comment argument in IO_Chain_Write/Append() - comment is initialized as dash "-" - OVF "valuedim" is now called by RequiredSingle() in Read_From_OVF() - renamed functions IO_Write_System_*() to IO_Chain_Write_*() - renamed function IO*System*() to IO*Image*() - extended API IO test - extended Python API IO test - modified python wrapping - modified corresponding documentation

GPMueller

Too many things are being done at once, we need to merge this soon and break up the remaining tasks into smaller PRs, or I will not be able to properly review all the changes.

GPMueller · 2017-11-13T08:55:24Z

core/src/io/Datawriter.cpp

+        // Data
+        output_to_file += fmt::format( "# Begin: data {}\n", datatype );
+
+        switch ( format ) {


Switch statements on the format give a warning such as 4 enumeration values not handled in switch: 'OVF_BIN8', 'OVF_BIN4', 'OVF_TEXT'... [-Wswitch]. When only 3 cases are treated it may be better to just use if/else.

GPMueller · 2017-11-13T08:57:20Z

core/src/engine/Method_GNEB.cpp

 			{
 				// File name
 				std::string chainFile = preChainFile + suffix + ".txt";

 				// Chain
-				IO::Save_SpinChain_Configuration(this->chain, iteration, chainFile);
-			};
+                std::string output_comment( "Iteration: " + iteration );


Rather write std::string output_comment = fmt::format("Iteration: {}", iteration));, see the warning this produces: warning: adding 'const int' to a string does not append to the string [-Wstring-plus-int]

GPMueller · 2017-11-13T09:06:11Z

core/include/Spirit/Exception.h

@@ -0,0 +1,9 @@
+// This is provided for homogeneous exception handling between core and C++ UIs


I removed this file with e26d546, it should be removed on this branch, too, as no exception should ever pass the API layer.

GPMueller · 2017-11-13T09:12:58Z

core/include/data/Parameters_Method.hpp

-			long int n_iterations, long int n_iterations_log, long int max_walltime_sec,
-			std::shared_ptr<Pinning> pinning, scalar force_convergence);
+		Parameters_Method(std::string output_folder, std::string output_file_tag,
+            std::array<bool,3> output, long int n_iterations, long int n_iterations_log, 


You said that bundlling some of the parameters toghether, which are passed into several constructors may be a good idea. I guess the config of the output would be a good place to start - something along the lines of:

struct Output_Config { std::string folder; bool any; bool initial; bool final; }

This would shorten the constructors of the parameters and make them clearer.

Note: indentation in the Parameters files is messed up, please normalize it to spaces.

GPMueller · 2017-11-14T16:41:45Z

core/src/engine/Method_GNEB.cpp

-			}
+            std::string fileTag;
+
+            if (this->systems[0]->llg_parameters->output_file_tag == "<time>")


As the tag has been abstracted as a parameter across all Methods, it would make sense to get this from a function.
However, it's important to use the child class Parameters member, (i.e. gneb_parameters in the GNEB case)!
At least the latter should be fixed in this PR.

GPMueller · 2017-11-14T16:45:33Z

core/src/io/Datawriter.cpp

@@ -54,10 +58,11 @@ namespace IO
 		if (readability_toggle) header = separator + line + separator;
 		else header = line;
 		if (!readability_toggle) std::replace( header.begin(), header.end(), '|', ' ');
-		String_to_File(header, fileName);


All the formatting changes (indentation, where you break lines etc... and now this kind of change... You do too much of this at once! This PR is very hard to keep track of, as you pile a lot of these aesthetic changes on top of the refactoring. Please try to keep this to a minimum and rather create another PR later, where you only do clean-up.

- Implemented OVF binary 4 - OVF binary writing independently of scalar length

- Corrected comments in Datawriter for OVF - Added all the OVF formats in the unit test

- Some switch statements converted to ifs - Changed buffer to array when reading and writing OVF - Changed output_comment declaration in Method_GNEB.cpp

GPMueller

I would say this PR is ready to be merged.
A few issues can be closed with this and the remaining should be tackled with further PRs.

Implementation of simulation identifier tag

3dda49b

- Modified the Parameters_Method_* and Method_* source files - Modified Logging, State, Configparser and Configwriter source files - Modified the default input file and the input files of the unit tests - Modified the INPUT documentation

GPMueller self-requested a review October 4, 2017 14:21

GPMueller assigned smavros Oct 4, 2017

GPMueller added core enhancement feature-request labels Oct 4, 2017

Test for OVF file IO

f629c32

- Added IO test - Improved OVF writer - Added IO files of unit tests to gitignore - Added gitkeep in IO folder of unit tests

GPMueller force-pushed the develop branch 22 times, most recently from 4067c02 to 54cf96f Compare October 8, 2017 13:36

Improvements

f9923b1

- Improved IO test - Added Require_Single() method in Filter_File_Handle

GPMueller mentioned this pull request Oct 25, 2017

Core IO: add optional comment string to API and Datawriter functions #338

Closed

smavros added 2 commits October 27, 2017 13:12

Merge remote-tracking branch 'upstream/develop' into feature-IO-impro…

b7866c7

…vements

smavros force-pushed the feature-IO-improvements branch from 485474f to b7866c7 Compare October 27, 2017 13:12

smavros and others added 4 commits October 27, 2017 15:42

Implemented API writting with optional comment spirit-code#166 spirit…

10037fb

…-code#338

Reimplemented Save_To_OVF() for

5efec4a

- rectangular mesh - 8byte binary - text - Split the data writing in different functions

Moved Version.h from Spirit to utility

8a99b04

Merge 'upstream/develop' into 'feature-IO-improvements'.

56b7b33

smavros mentioned this pull request Nov 8, 2017

File IO: SIGSEGV from IO::Write_Image_Energy_per_Spin() #342

Closed

smavros and others added 2 commits November 11, 2017 16:29

Merge branch 'develop' into feature-IO-improvements

3763cb5

GPMueller force-pushed the feature-IO-improvements branch from f16d8d0 to 3763cb5 Compare November 13, 2017 08:41

GPMueller reviewed Nov 14, 2017

View reviewed changes

smavros added 2 commits November 15, 2017 10:59

Finished OVF binary writing

accb300

- Implemented OVF binary 4 - OVF binary writing independently of scalar length

Finished OVF binary parser for every case

0d035dd

- Corrected comments in Datawriter for OVF - Added all the OVF formats in the unit test

smavros force-pushed the feature-IO-improvements branch 2 times, most recently from d52ea41 to 4b4e9d3 Compare November 15, 2017 17:37

Corrections in Datawriter.cpp

88989c6

- Some switch statements converted to ifs - Changed buffer to array when reading and writing OVF - Changed output_comment declaration in Method_GNEB.cpp

smavros force-pushed the feature-IO-improvements branch from 4b4e9d3 to 88989c6 Compare November 15, 2017 19:04

Core: Fixed small but critical bug in Datawriter.

cb946a1

GPMueller approved these changes Nov 16, 2017

View reviewed changes

GPMueller merged commit 0b9db55 into spirit-code:develop Nov 16, 2017

This was referenced Nov 16, 2017

Core: add OVF file format support #193

Closed

Write Spirit version as comment into output files #166

Closed

Core: input parser should ignore comments after keywords #234

Closed

I/O file format specifications #31

Closed

smavros deleted the feature-IO-improvements branch November 20, 2017 20:57

GPMueller mentioned this pull request Nov 28, 2017

Core: improve file IO #220

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File IO improvements #319

File IO improvements #319

smavros commented Oct 4, 2017

GPMueller commented Oct 4, 2017

smavros commented Oct 25, 2017 •

edited

Loading

GPMueller left a comment

GPMueller Nov 13, 2017

GPMueller Nov 13, 2017

GPMueller Nov 13, 2017

GPMueller Nov 13, 2017

GPMueller Nov 14, 2017

GPMueller Nov 14, 2017

GPMueller left a comment

		@@ -0,0 +1,9 @@
		// This is provided for homogeneous exception handling between core and C++ UIs

File IO improvements #319

File IO improvements #319

Conversation

smavros commented Oct 4, 2017

GPMueller commented Oct 4, 2017

smavros commented Oct 25, 2017 • edited Loading

GPMueller left a comment

Choose a reason for hiding this comment

GPMueller Nov 13, 2017

Choose a reason for hiding this comment

GPMueller Nov 13, 2017

Choose a reason for hiding this comment

GPMueller Nov 13, 2017

Choose a reason for hiding this comment

GPMueller Nov 13, 2017

Choose a reason for hiding this comment

GPMueller Nov 14, 2017

Choose a reason for hiding this comment

GPMueller Nov 14, 2017

Choose a reason for hiding this comment

GPMueller left a comment

Choose a reason for hiding this comment

smavros commented Oct 25, 2017 •

edited

Loading