This bugzilla service is closed. All entries have been migrated to https://gitlab.com/libeigen/eigen

Bug 622

Summary: std::istream operator>> should be added
Product: Eigen Reporter: Helmut Jarausch <jarausch>
Component: GeneralAssignee: Nobody <eigen.nobody>
Status: DECISIONNEEDED ---    
Severity: Feature Request CC: chtz, gael.guennebaud, jacob.benoit.1, jdh8, rhys.ulerich, tobias.wood
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 209, 814    

Description Helmut Jarausch 2013-06-26 09:26:47 UTC
There should be an input operator>> which can read files written with the
built-in output operator<<

The following addition to Eigen/src/Core/IO.h would be quite helpful

+template<typename Derived>
+std::istream & operator >>
+(std::istream & s,
+ MatrixBase<Derived> & m)
+{
+    for (int i = 0; i < m.rows(); ++i)
+    for (int j = 0; j < m.cols(); j++)
+      s >> m(i,j);
+  
+  return s;
+}
 
 } // end namespace Eigen

IMHO there is no need for a fancy input operator (using non-white-space separators, e.g.) since the standard output operator doesn't create such files.

In teaching C++ using Eigen, it's so much easier to tell the students that
a matrix/vector written with operator<< can be read in by this new operator>>

Thanks for considering this tiny addition,

Helmut Jarausch
Lehrst. f. Numerische Mathematik
RWTH-Aachen, Germany
Comment 1 Christoph Hertzberg 2013-07-01 13:53:41 UTC
Blocking 3.3 in order to not postpone it again.

For reference here is a previous mailing list discussion about that:
http://listengine.tuxfamily.org/lists.tuxfamily.org/eigen/2010/07/msg00285.html
Comment 2 Christoph Hertzberg 2013-07-17 15:17:07 UTC
*** Bug 75 has been marked as a duplicate of this bug. ***
Comment 3 Christoph Hertzberg 2014-07-21 13:19:02 UTC
Chen-Pang made a pull-request on this feature here:
https://bitbucket.org/eigen/eigen/pull-request/73/

We still need to decide if the input operator shall resize the matrix (the above pull-request does not, and Helmut also seems not to require it). If there is general agreement, that it should not (or does not have to) I can live with that.

Another (independent) issue is what shall happen for invalid inputs. I don't think assertions are a good idea. Standard C++ behavior would be to mark the stream as bad and provide an undefined value (or don't change the value).
Also, should we make it possible to accept a mixture of separators? (I admit that tends to over-engineer the problem)
Comment 4 Helmut Jarausch 2014-07-21 13:38:35 UTC
(In reply to Christoph Hertzberg from comment #3)
> Chen-Pang made a pull-request on this feature here:
> https://bitbucket.org/eigen/eigen/pull-request/73/
> 
> We still need to decide if the input operator shall resize the matrix (the
> above pull-request does not, and Helmut also seems not to require it). If
> there is general agreement, that it should not (or does not have to) I can
> live with that.

I'm strongly against the option to resize the matrix since this will  
disable a means of error checking.

> 
> Another (independent) issue is what shall happen for invalid inputs. I don't
> think assertions are a good idea. Standard C++ behavior would be to mark the
> stream as bad and provide an undefined value (or don't change the value).

I'm strongly in favour of marking the stream as bad since this allows  
throwing an exception (which can be caught) by invoking exceptions(ios::failbit)  
for the stream.

> Also, should we make it possible to accept a mixture of separators? (I admit
> that tends to over-engineer the problem)

Another great opportunity to defer errors caused by invalid data.
I agree with Chen-Pang that valid inputs are (only) those which can be  
written by Eigen.
Comment 5 Gael Guennebaud 2014-07-21 15:06:51 UTC
Since we know that we are looking for a list of numbers with predefined length, separators are useless, and I'd like that by default operator>> skip anything that is not a number. At least we could pre-define a list of usual separators and sip any of them between each coefficient (maybe this a bit more tricky for complexes).

Aggressive checks would only be enabled when a IOFormat is explicitly passed. Moreover, IOFormat seems to be overkill for that purpose. Perhaps we could have a InFormat where we would only specify coefficient and row separators?

Automatic resizing seems to be quite appealing too. Auto-resizing could be enabled by default for null matrices or enabled explicitly via an option of InFormat.

At the end this feature is mostly useful for debugging and prototyping, not for production code for which efficient binary formats have to be used anyway. So overall, I think that we should favor flexibility to ease exchanging data between softwares. Typically I'd like that:

MatrixXd A;
stream >> A;

works out-of-the-box regardless of the actual formatting of the data.
Comment 6 Chen-Pang He 2014-07-21 15:58:44 UTC
Now I mark istream as failed when loading invalid data.
https://bitbucket.org/jdh8/eigen/commits/95bc93623f7ef5a2ac0d84dc6cd227dcf0df2d25
Comment 7 Christoph Hertzberg 2014-07-21 16:13:26 UTC
(In reply to Gael Guennebaud from comment #5)
> Aggressive checks would only be enabled when a IOFormat is explicitly
> passed. Moreover, IOFormat seems to be overkill for that purpose. Perhaps we
> could have a InFormat where we would only specify coefficient and row
> separators?

The name "IOFormat" kind of suggests that it defines both input and output. Of course, we could introduce separate classes for both (and maybe rename IOFormat to OutFormat, keeping IOFormat as alias).

> At the end this feature is mostly useful for debugging and prototyping, not
> for production code for which efficient binary formats have to be used
> anyway. So overall, I think that we should favor flexibility to ease
> exchanging data between softwares. Typically I'd like that:
> 
> MatrixXd A;
> stream >> A;
> 
> works out-of-the-box regardless of the actual formatting of the data.

Yes, that's what I'd expect to work if we want to support istream>>MatrixXd.

If we don't explicitly provide formatting, we could make "\n" or ";" the line separator and all other whitespace or "," the element separator. End-Of-Matrix could be indicated by "\n\n" or by EOF/end-of-stream. With a tiny bit of effort, we could also accept matching pairs of parenthesis as start/end markers.

Of course, implementing that is still not very straight-forward, as it requires some rudimentary consistency checks (mostly that all lines are of equal length). And, as you mentioned, inputting complex matrices is another thing we'd need to address ...
Comment 8 Chen-Pang He 2014-07-22 05:13:43 UTC
I agree to be more liberal if no format is specified.  If not resizing, we can even skip all NaNs by checking istream::rdstate()
Comment 9 Helmut Jarausch 2014-07-22 11:07:16 UTC
(In reply to Christoph Hertzberg from comment #3)
> Chen-Pang made a pull-request on this feature here:
> https://bitbucket.org/eigen/eigen/pull-request/73/
> 

In its current form this is broken.
The following test fails.
If NDEBUG isn't set an assertion is spit out
otherwise the 3rd inputed  matrix differs from the original.

#include <iostream>
using std::endl;  using std::cout;
#include <Eigen/Core>
using Eigen::IOFormat;
#include <sstream>
using std::istringstream;
using std::ostringstream;

int main() {
  Eigen::Matrix3d m1;
  m1 << 1.111111, 2, 3.33333, 4, 5, 6, 7, 8.888888, 9;

  IOFormat CommaInitFmt(Eigen::StreamPrecision, Eigen::DontAlignCols, ", ", ", ", "", "", " << ", ";");
  IOFormat CleanFmt(4, 0, ", ", "\n", "[", "]");
  IOFormat OctaveFmt(Eigen::StreamPrecision, 0, ", ", ";\n", "", "", "[", "]");
  IOFormat HeavyFmt(Eigen::FullPrecision, 0, ", ", ";\n", "[", "]", "[", "]");
  ostringstream Out;
  Out  << m1 << endl;
  Out  << m1.format(CommaInitFmt) << endl;
  Out  << m1.format(CleanFmt) << endl;
  Out  << m1.format(OctaveFmt) << endl;
  Out  << m1.format(HeavyFmt) << endl;
  istringstream Inp(Out.str());
  Eigen::Matrix3d verify;
  Inp >> verify;
  if ( (m1-verify).norm() > 1E-4 ) cout << "\n*** FAILED ***\n" << verify << endl;
  else                             cout << "\n OK : " << (m1-verify).norm() << endl;
  Inp >> verify.format(CommaInitFmt);
  if ( (m1-verify).norm() > 1E-4 ) cout << "\n*** FAILED ***\n" << verify << endl;
  else                             cout << "\n OK : " << (m1-verify).norm() << endl;
  Inp >> verify.format(CleanFmt);    
  if ( (m1-verify).norm() > 1E-4 ) cout << "\n*** FAILED ***\n" << verify << endl;
  else                             cout << "\n OK : " << (m1-verify).norm() << endl;
  Inp >> verify.format(OctaveFmt);   
  if ( (m1-verify).norm() > 1E-4 ) cout << "\n*** FAILED ***\n" << verify << endl;
  else                             cout << "\n OK : " << (m1-verify).norm() << endl;
  Inp >> verify.format(HeavyFmt);    
  if ( (m1-verify).norm() > 1E-4 ) cout << "\n*** FAILED ***\n" << verify << endl;
  else                             cout << "\n OK : " << (m1-verify).norm() << endl;
}
Comment 10 Chen-Pang He 2014-07-22 13:13:57 UTC
It failed becuase the linefeeds between matrices invalidate the input.  Ignoring whitespaces before matrix prefix whould fix this.
https://bitbucket.org/jdh8/eigen/commits/41bde9b6d69da484c22d9f564bbdc92bd41b14e6

We can even ignore more whitespaces by trimming separators and having opterator>>(istream&, skip) check the trimmed input.

{
  Make the istream ignore whitespaces.  (std::skipws)

  if (The trimmed separator is nonempty)
  {
    Pour the trimmed input into a buffer.
    Compare the buffer and the separator.
  }

  Set the status back.  (whether the istream ignores whitespaces)
}
Comment 11 Chen-Pang He 2014-07-22 14:34:43 UTC
https://bitbucket.org/jdh8/eigen/commits/3c8e3105c47482c942a75019305b1c95061cb4ce

Make istream noskipws if DontAlignCols set, skipws if unset.

Whitespaces before matrix prefix are still always skipped with std::ws.
Whitespaces in a matrix are skipped iff DontAlignCols unset.
Comment 12 Helmut Jarausch 2014-07-23 09:36:07 UTC
(In reply to Gael Guennebaud from comment #5)
> Since we know that we are looking for a list of numbers with predefined
> length, separators are useless, and I'd like that by default operator>> skip
> anything that is not a number. At least we could pre-define a list of usual
> separators and sip any of them between each coefficient (maybe this a bit
> more tricky for complexes).

This is impossible!
Any clean implementation should enable normal (i.e. non-Eigen) input between
input to Eigen objects. Therefore Eigen's input operator must leave the input stream in a healthy state. 
It's definitely possible to skip any 'disturbing' characters *BEFORE* a number
which is read by Eigen's input operator, but it is impossible to know what to
skip of the trailing characters *AFTER* the last number input by Eigen's operator.

So, the only means to do that right, is to go along the lines of Chen-Pang.
Eigen's input operator must know which trailing delimiters belong to Eigen's input and which characters must be left in the stream for a following non-Eigen input operator.
Comment 13 Gael Guennebaud 2014-10-20 10:04:46 UTC
In reply to comment 12:

In my opinion, the main use case is to read a single matrix from a string or file. In that context, I see operator<< like matlab's load function which is pretty flexible regarding the structuring format, hence the default behavior I suggested in comment 5.

Enabling operator>> to resize the matrix is also mandatory, otherwise you would have to figure out the matrix sizes by parsing the file yourself making the availability of an operator>> pointless. Of course, if the sizes are known in advance, it might be convenient to disable automatic resizing. API-wise, a ugly workaround would be to let the user use a Block, e.g.:

  stream >> A.leftCols(A.cols());

but if we agree that we need a new InFormat, then this option could be handled by this new class.


A second typical use case is to load small fixed size matrices from a configuration or scene description file. In that case, since the sizes are known at compile time, the suggested default behavior is still OK.


For more advanced usage, first I would argue that operator<< and operator>> are not the best choice for serialization purpose, and a binary format has to be preferred. Anyway, if we put aside this remark, since our default format for operator<< does not have begin/end delimiters you/we have only three options:

 1 - user side: figure-out the size in advance (e.g., from the stream) *and* disable automatic resizing
 2 - user side: use a custom format with clear begin/end separators
 3 - Eigen's side: have a different format for in and out operations

The third option is not very welcome, and so in both first two cases you cannot simply do:

  MatrixXd A;
  stream >> A;

and so you cannot rely on the default behavior regardless of how it is implemented. In other words, the flexible default behavior has to be by-passed whatsoever and you're good regarding all the possible troubles you pointed-out.
Comment 14 Christoph Hertzberg 2014-10-20 11:39:31 UTC
(In reply to Gael Guennebaud from comment #13)
> In my opinion, the main use case is to read a single matrix from a string or
> file. In that context, I see operator<< like matlab's load function which is
> pretty flexible regarding the structuring format, hence the default behavior
> I suggested in comment 5.

Yes, I think being (mostly) compatible with Matlab's input syntax should be desired.

> API-wise, a ugly workaround would be to let the user use a Block, e.g.:
> 
>   stream >> A.leftCols(A.cols());

A slightly nicer (but also not very obvious) syntax could be:

   stream >> A.matrix(); // or A.array()

> but if we agree that we need a new InFormat, then this option could be
> handled by this new class.

Yes, I think an InFormat class would be nice to have if only custom formats shall be accepted.

> [...] Anyway, if we put aside this remark, since our default
> format for operator<< does not have begin/end delimiters you/we have only
> three options:
> 
>  1 - user side: figure-out the size in advance (e.g., from the stream) *and*
> disable automatic resizing
>  2 - user side: use a custom format with clear begin/end separators
>  3 - Eigen's side: have a different format for in and out operations

We can make "\n" the default line separator and END_OF_STREAM or "\n\n" the default end-of-matrix marker. Alternatively, we can stop inputting a matrix as soon as something not interpretable as a number (after skipping possible whitespace and separators) appears.
This should make the following work by default (neglecting stream precision):

  Matrix Ain(initialized, somehow); Aout;
  ostringstream out; out << Ain;
  istringstream in(out.str()); in >> Aout;
  VERIFY_EQUALS(Ain, Aout);
Comment 16 Gael Guennebaud 2014-10-20 13:35:15 UTC
(In reply to Christoph Hertzberg from comment #14)
> A slightly nicer (but also not very obvious) syntax could be:
> 
>    stream >> A.matrix(); // or A.array()

This cannot work as A.matrix() returns a reference to A if A is already a MatrixBase.

> We can make "\n" the default line separator and END_OF_STREAM or "\n\n" the
> default end-of-matrix marker. Alternatively, we can stop inputting a matrix
> as soon as something not interpretable as a number (after skipping possible
> whitespace and separators) appears.
> This should make the following work by default (neglecting stream precision):
> 
>   Matrix Ain(initialized, somehow); Aout;
>   ostringstream out; out << Ain;
>   istringstream in(out.str()); in >> Aout;
>   VERIFY_EQUALS(Ain, Aout);


Yes, of course we should not try to skip everything and eat all the numbers we can find, but skip only a set of predefined common separators, something like:

matrix start: B*S?B*
line start:   B*S?B*
coeff sep:    B*(B|,)B*
line end:     B*((E|;)B*$?|$)
matrix end:   B*(E?|$B*$|EOF)

using the following shortcuts:

S := [\(\{\[]      // Start
E := [\)\}\]]      // End
B := [ \t]         // Blank
$ := \n|\r|\n\r    // end-of-line

Of course, the symbols picked within the S and E sets should match respectively.
Comment 17 Helmut Jarausch 2014-10-20 14:04:37 UTC
(In reply to Gael Guennebaud from comment #13)
> In reply to comment 12:
> 
> In my opinion, the main use case is to read a single matrix from a string or
> file. In that context, I see operator<< like matlab's load function which is
> pretty flexible regarding the structuring format, hence the default behavior
> I suggested in comment 5.
> 
> Enabling operator>> to resize the matrix is also mandatory, otherwise you
> would have to figure out the matrix sizes by parsing the file yourself
> making the availability of an operator>> pointless. Of course, if the sizes
> are known in advance, it might be convenient to disable automatic resizing.
> API-wise, a ugly workaround would be to let the user use a Block, e.g.:
> 
>   stream >> A.leftCols(A.cols());
> 
> but if we agree that we need a new InFormat, then this option could be
> handled by this new class.
> 
> 
> A second typical use case is to load small fixed size matrices from a
> configuration or scene description file. In that case, since the sizes are
> known at compile time, the suggested default behavior is still OK.
> 
> 

To me, the ability to resize a matrix is not mandatory.
I've always taught my students to do something like

#include <sstream>
using std::istringstream;  using std::ostringstream;
using std::endl;

#include <Eigen/Core>
using Eigen::MatrixXd;

int main() {
  MatrixXd A(3,2);
  A << 1, 2,
       3, 4,
       5, 6;

  ostringstream OUT;
  OUT << A.rows() << ' ' << A.cols() << endl << A << endl;
  
  istringstream Inp(OUT.str());
  int A_rows, A_Cols;
  Inp >> A_rows >> A_Cols;
  MatrixXd AC(A_rows, A_Cols);
  Inp >> AC;
  return 0;
}

I have just the requirement that any output which is produced by Eigen should
be able to be read back by Eigen.
If a user is able to (and does) specify special delimiters (like [] in Matlab)
during output, he/she should  specify the same delimiters on input.
Of course, in that case the ability to resize the matrix is necessary.

Chen-Pang's solution enables my use case. The second use case is appreciated 
if possible in time.

Stopping input on any character which is not allowed for a valid float value
is a bad solution since it hides errors. Either the user knows the dimension
of the matrix beforehand or he/she must specify the correct delimiters which
have been used to create the output.

Thanks for discussing these points,
Helmut
Comment 18 Tobias Wood 2014-10-20 14:59:49 UTC
Hello,
My comments as a user instead of a developer are:
1) operator<< and operator>> should be symmetric. Anything else is needlessly confusing.
2) The Matlab 'load' operator (and 'save') is not restricted to loading a single matrix from a file, it can be used for entire workspace, containing multiple matrices/variables of any kind, so I don't think Comment 12 is a fair comparison.
3) My personal use case for operator>> would be to read many dynamic small matrices/arrays at run-time. Currently I ask the user to specify how large they are and then read them manually.

Given the above my preference is for a very simple operator>> that mirrors the current operator<<, and needs to know the matrix size in advance. Anything else would require at least some changes to operator<< in order to be consistent, for instance in my opinion introducing an operator>> that is capable of auto-resizing would also require introducing standard delimiters (most likely []) to operator<<.
Comment 19 Christoph Hertzberg 2014-10-20 15:11:47 UTC
(In reply to Tobias Wood from comment #18)
> 1) operator<< and operator>> should be symmetric. Anything else is
> needlessly confusing.
> 2) The Matlab 'load' operator (and 'save') is not restricted to loading a
> single matrix from a file, it can be used for entire workspace, containing
> multiple matrices/variables of any kind, so I don't think Comment 12 is a
> fair comparison.
> 3) My personal use case for operator>> would be to read many dynamic small
> matrices/arrays at run-time. Currently I ask the user to specify how large
> they are and then read them manually.

IMO this contradicts your point 1) Having to manually resize the matrix is counter-intuitive.
Regarding 2) I agree that advanced Matlab syntax such as
   [[11;21], [12 13; 22, 23]]
for
 11 12 13
 21 22 23
should not be required for a first implementation -- this can easily added in later versions. 

However, automatically resizing matrices (by default) is a feature we need to decide about now, or users may experience strange behavior if this is added in later versions.
At least Helmut, Cheng-Pang and you (Tobias) do not seem to require this, but Gael and I (Christoph) think it should be added. Any other opinions?
Comment 20 Gael Guennebaud 2014-10-20 15:44:32 UTC
Alright, soi it seems to me that the only remaining issue is whether we should resize by default or not.


With automatic resizing by default, then Helmut's example would require a small change to work. For instance, as suggested by Christoph, if we include an empty line as a default matrix separator, then we can workaround by requiring the user to manually insert two line breaks, e.g.:

cout << A << endl << endl;

The first one is mandatory to avoid gluing numbers, and the second most recommended anyway to make the stream human readable.


If we don't resize by default, then we need to figure out an API to enable auto-resizing...


A third option, would be to enable automatic resizing if the matrix is empty (i.e., 0x0) and no resizing otherwise. This should work seamlessly most of the time with a possible behavior ambiguity if one fills an already filled matrix while expecting an automatic change of sizes. In that case the user has to call A.resize(0,0), before filling it. No big deal, and that's a pretty rare use case IMO. This strategy also has the advantage to avoid the need to invent a cumbersome API to tell whether we want or not resizing.
Comment 21 Gael Guennebaud 2014-10-20 15:47:10 UTC
(In reply to Gael Guennebaud from comment #20)
> A third option, would be to enable automatic resizing if the matrix is empty
> [...]

btw, if we agree with that strategy, then we could also apply it to the comma initializer syntax (only for matrices having one fixed and one dynamic dimension such as vectors).
Comment 22 Tobias Wood 2014-10-20 16:05:03 UTC
Auto-resizing empty matrices only sounds sensible.

One further issue I just wondered about is how you are intending to buffer/resize the read of an auto-resized matrix, if someone is masochistic enough to do it with a large matrix? The auto-resize-only-if-empty would let someone read in a large dynamic matrix without any temporary allocation.

The double endl idea sounds workable but there is a corner case where it might be counter-intuituve - reading row vectors from stdin (similar to my use case). Would it be waived in this case?

(Sorry if I now sound keen on the idea of auto-resizing. I should have made it clear that my objections were on practical grounds and in an ideal world it would be nice to have)
Comment 23 Gael Guennebaud 2014-10-20 17:01:32 UTC
(In reply to Tobias Wood from comment #22)
> Auto-resizing empty matrices only sounds sensible.

Great.

> One further issue I just wondered about is how you are intending to
> buffer/resize the read of an auto-resized matrix, if someone is masochistic
> enough to do it with a large matrix? The auto-resize-only-if-empty would let
> someone read in a large dynamic matrix without any temporary allocation.

That's an implementation details and we have the same issue for the loadMarket functions. I see three options:
1 - naively realloc on demand like std::vector::push_back
2 - parse the stream twice (only possible if you're parsing a stored file)
3 - store the data in a linked-list of chunk and then copy everything once done.

I guess we'll start by (1) and implement (3) if someone is willing to do it.

> The double endl idea sounds workable but there is a corner case where it
> might be counter-intuituve - reading row vectors from stdin (similar to my
> use case). Would it be waived in this case?

Should be fine because one dimension should be known at compile-time. So the double endl should not be needed. The double endl would be needed only if it's not possible to determine the end of the matrix/vector by other means (number of rows or other explicit delimiters).
Comment 24 Christoph Hertzberg 2014-10-20 21:21:00 UTC
(In reply to Gael Guennebaud from comment #23)
> (In reply to Tobias Wood from comment #22)
> > Auto-resizing empty matrices only sounds sensible.
> Great.

I agree that's a good compromise.

> > [...] buffer/resize [...] large matrix? [...]
> 
> That's an implementation details and we have the same issue for the
> loadMarket functions. I see three options:
> 1 - naively realloc on demand like std::vector::push_back
> 2 - parse the stream twice (only possible if you're parsing a stored file)
> 3 - store the data in a linked-list of chunk and then copy everything once
> done.

I guess (3) but simply taking a std::deque should be good enough. I guess the temporary should be stored row-major anyways (as that is the order it is read in). As soon as the end-of-matrix is detected, the destination matrix is resized once and the values are copied (later enhancement: for big-data types moved) to the matrix one-by-one.
As has been mentioned several times, efficiency shouldn't be a big concern, because users who want performance shall use some kind of binary format anyways.

> > The double endl idea sounds workable but there is a corner case where it
> > might be counter-intuituve - reading row vectors from stdin (similar to my
> > use case). Would it be waived in this case?
> 
> Should be fine because one dimension should be known at compile-time. So the
> double endl should not be needed. The double endl would be needed only if
> it's not possible to determine the end of the matrix/vector by other means
> (number of rows or other explicit delimiters).

Agree. If the number of rows is fixed, we don't need to determine the last line of the matrix (but the length).


The only decision I see open, is what to do if, e.g., the number of elements per row is inconsistent, or for not-to-be-resized matrices the number of elements is not sufficient. (1,2,3 and A,B,C are orthogonal):

 1) Make sure the matrix is not changed (will require a buffer in every case)
 2) Store as many entries in the matrix as the input was consistent
 3) Have the matrix in an undefined state (e.g. some elements may have been written already)
 A) Read from the stream until something unexpected appears, mark the stream as bad.
 B) Try to unget/putback all characters that don't fit consistently to the matrix size, mark the stream as bad.
 C) Undefined contents of the stream, but stream is marked bad.

I'll tend to 3+C and have the user take care of trying to reset the contents/state of the stream if he wants more graceful error recovery.
Comment 25 Gael Guennebaud 2014-10-20 22:54:00 UTC
(In reply to Christoph Hertzberg from comment #24)
> The only decision I see open, is what to do if, e.g., the number of elements
> per row is inconsistent, or for not-to-be-resized matrices the number of
> elements is not sufficient. (1,2,3 and A,B,C are orthogonal):
> 
>  1) Make sure the matrix is not changed (will require a buffer in every case)
>  2) Store as many entries in the matrix as the input was consistent
>  3) Have the matrix in an undefined state (e.g. some elements may have been
> written already)
>  A) Read from the stream until something unexpected appears, mark the stream
> as bad.
>  B) Try to unget/putback all characters that don't fit consistently to the
> matrix size, mark the stream as bad.
>  C) Undefined contents of the stream, but stream is marked bad.
> 
> I'll tend to 3+C and have the user take care of trying to reset the
> contents/state of the stream if he wants more graceful error recovery.

I'm not sure that in practice there is a difference between A and C: A is how it's implemented, and C how it is documented ;) Anyway, 3+C is good to me as the other alternatives might be too tricky to get them right and clearly documented.
Comment 26 Nobody 2019-12-04 12:26:31 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to gitlab.com's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.com/libeigen/eigen/issues/622.