Skip to content
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

Update documentation.py #23

Closed
wants to merge 1 commit into from
Closed

Conversation

MartyIX
Copy link

@MartyIX MartyIX commented Feb 1, 2017

Hopefully fixes/improves #21 issue.

I have used the following repo: https://github.com/Reference-LAPACK/lapack as the data source, I'm not sure if it is a correct one though. However, it should be documented somewhere. :-)

I ran:

python3 c.py ../.data/lapack-master >new.result

and I got the following diff:

27,29c27,30
< /// Returns D.P. Dot product accumulated in D.P., for S.P. SX and SY DSDOT = sum
< /// for I = 0 to N-1 of SX(LX+I*INCX) * SY(LY+I*INCY), where LX = 1 if INCX .GE.
< /// 0, else LX = 1+(1-N)*INCX, and LY is defined in a similar way using INCY.
---
> /// Returns D.P. dot product accumulated in D.P., for S.P. SX and SY
> /// DSDOT = sum for I = 0 to N-1 of  SX(LX+I*INCX) * SY(LY+I*INCY),
> /// where LX = 1 if INCX .GE. 0, else LX = 1+(1-N)*INCX, and LY is
> /// defined in a similar way using INCY.
312,321c313,324
< /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS THE SECOND
< /// COMPONENT OF THE 2-VECTOR (SQRT(SD1)*SX1,SQRT(SD2)*> SY2)**T. WITH
< /// SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// SFLAG=-1.E0 SFLAG=0.E0 SFLAG=1.E0 SFLAG=-2.E0
< ///
< ///  (SH11 SH12) (1.E0 SH12) (SH11 1.E0) (1.E0 0.E0) H=() () () () (SH21 SH22),
< /// (SH21 1.E0), (-1.E0 SH22), (0.E0 1.E0). LOCATIONS 2-4 OF SPARAM CONTAIN
< /// SH11,SH21,SH12, AND SH22 RESPECTIVELY. (VALUES OF 1.E0, -1.E0, OR 0.E0
< /// IMPLIED BY THE VALUE OF SPARAM(1) ARE NOT STORED IN SPARAM.)
---
> /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS
> /// THE SECOND COMPONENT OF THE 2-VECTOR  (SQRT(SD1)*SX1,SQRT(SD2)*>    SY2)**T.
> /// WITH SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// SFLAG=-1.E0     SFLAG=0.E0        SFLAG=1.E0     SFLAG=-2.E0
> ///
> ///   (SH11  SH12)    (1.E0  SH12)    (SH11  1.E0)    (1.E0  0.E0)
> /// H=(          )    (          )    (          )    (          )
> ///   (SH21  SH22),   (SH21  1.E0),   (-1.E0 SH22),   (0.E0  1.E0).
> /// LOCATIONS 2-4 OF SPARAM CONTAIN SH11,SH21,SH12, AND SH22
> /// RESPECTIVELY. (VALUES OF 1.E0, -1.E0, OR 0.E0 IMPLIED BY THE
> /// VALUE OF SPARAM(1) ARE NOT STORED IN SPARAM.)
345,353c348,357
< /// SX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE LX =
< /// (-INCX)*N, AND SIMILARLY FOR SY USING USING LY AND INCY. WITH
< /// SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// SFLAG=-1.E0 SFLAG=0.E0 SFLAG=1.E0 SFLAG=-2.E0
< ///
< ///  (SH11 SH12) (1.E0 SH12) (SH11 1.E0) (1.E0 0.E0) H=() () () () (SH21 SH22),
< /// (SH21 1.E0), (-1.E0 SH22), (0.E0 1.E0). SEE SROTMG FOR A DESCRIPTION OF DATA
< /// STORAGE IN SPARAM.
---
> /// SX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE
> /// LX = (-INCX)*N, AND SIMILARLY FOR SY USING USING LY AND INCY.
> /// WITH SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// SFLAG=-1.E0     SFLAG=0.E0        SFLAG=1.E0     SFLAG=-2.E0
> ///
> ///   (SH11  SH12)    (1.E0  SH12)    (SH11  1.E0)    (1.E0  0.E0)
> /// H=(          )    (          )    (          )    (          )
> ///   (SH21  SH22),   (SH21  1.E0),   (-1.E0 SH22),   (0.E0  1.E0).
> /// SEE  SROTMG FOR A DESCRIPTION OF DATA STORAGE IN SPARAM.
369,378c373,384
< /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS THE SECOND
< /// COMPONENT OF THE 2-VECTOR (DSQRT(DD1)*DX1,DSQRT(DD2)*> DY2)**T. WITH
< /// DPARAM(1)=DFLAG, H HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// DFLAG=-1.D0 DFLAG=0.D0 DFLAG=1.D0 DFLAG=-2.D0
< ///
< ///  (DH11 DH12) (1.D0 DH12) (DH11 1.D0) (1.D0 0.D0) H=() () () () (DH21 DH22),
< /// (DH21 1.D0), (-1.D0 DH22), (0.D0 1.D0). LOCATIONS 2-4 OF DPARAM CONTAIN
< /// DH11, DH21, DH12, AND DH22 RESPECTIVELY. (VALUES OF 1.D0, -1.D0, OR 0.D0
< /// IMPLIED BY THE VALUE OF DPARAM(1) ARE NOT STORED IN DPARAM.)
---
> /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS
> /// THE SECOND COMPONENT OF THE 2-VECTOR  (DSQRT(DD1)*DX1,DSQRT(DD2)*>    DY2)**T.
> /// WITH DPARAM(1)=DFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// DFLAG=-1.D0     DFLAG=0.D0        DFLAG=1.D0     DFLAG=-2.D0
> ///
> ///   (DH11  DH12)    (1.D0  DH12)    (DH11  1.D0)    (1.D0  0.D0)
> /// H=(          )    (          )    (          )    (          )
> ///   (DH21  DH22),   (DH21  1.D0),   (-1.D0 DH22),   (0.D0  1.D0).
> /// LOCATIONS 2-4 OF DPARAM CONTAIN DH11, DH21, DH12, AND DH22
> /// RESPECTIVELY. (VALUES OF 1.D0, -1.D0, OR 0.D0 IMPLIED BY THE
> /// VALUE OF DPARAM(1) ARE NOT STORED IN DPARAM.)
402,410c408,417
< /// DX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE LX =
< /// (-INCX)*N, AND SIMILARLY FOR SY USING LY AND INCY. WITH DPARAM(1)=DFLAG, H
< /// HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// DFLAG=-1.D0 DFLAG=0.D0 DFLAG=1.D0 DFLAG=-2.D0
< ///
< ///  (DH11 DH12) (1.D0 DH12) (DH11 1.D0) (1.D0 0.D0) H=() () () () (DH21 DH22),
< /// (DH21 1.D0), (-1.D0 DH22), (0.D0 1.D0). SEE DROTMG FOR A DESCRIPTION OF DATA
< /// STORAGE IN DPARAM.
---
> /// DX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE
> /// LX = (-INCX)*N, AND SIMILARLY FOR SY USING LY AND INCY.
> /// WITH DPARAM(1)=DFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// DFLAG=-1.D0     DFLAG=0.D0        DFLAG=1.D0     DFLAG=-2.D0
> ///
> ///   (DH11  DH12)    (1.D0  DH12)    (DH11  1.D0)    (1.D0  0.D0)
> /// H=(          )    (          )    (          )    (          )
> ///   (DH21  DH22),   (DH21  1.D0),   (-1.D0 DH22),   (0.D0  1.D0).
> /// SEE DROTMG FOR A DESCRIPTION OF DATA STORAGE IN DPARAM.

Hopefully fixes/improves blas-lapack-rs#21 issue.

I have used the following repo: https://github.com/Reference-LAPACK/lapack as the data source, I'm not sure if it is a correct one though. However, it should be documented somewhere. :-)

I ran:

```
python3 c.py ../.data/lapack-master >new.result
```

and I got the following diff:

```
27,29c27,30
< /// Returns D.P. Dot product accumulated in D.P., for S.P. SX and SY DSDOT = sum
< /// for I = 0 to N-1 of SX(LX+I*INCX) * SY(LY+I*INCY), where LX = 1 if INCX .GE.
< /// 0, else LX = 1+(1-N)*INCX, and LY is defined in a similar way using INCY.
---
> /// Returns D.P. dot product accumulated in D.P., for S.P. SX and SY
> /// DSDOT = sum for I = 0 to N-1 of  SX(LX+I*INCX) * SY(LY+I*INCY),
> /// where LX = 1 if INCX .GE. 0, else LX = 1+(1-N)*INCX, and LY is
> /// defined in a similar way using INCY.
312,321c313,324
< /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS THE SECOND
< /// COMPONENT OF THE 2-VECTOR (SQRT(SD1)*SX1,SQRT(SD2)*> SY2)**T. WITH
< /// SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// SFLAG=-1.E0 SFLAG=0.E0 SFLAG=1.E0 SFLAG=-2.E0
< ///
< ///  (SH11 SH12) (1.E0 SH12) (SH11 1.E0) (1.E0 0.E0) H=() () () () (SH21 SH22),
< /// (SH21 1.E0), (-1.E0 SH22), (0.E0 1.E0). LOCATIONS 2-4 OF SPARAM CONTAIN
< /// SH11,SH21,SH12, AND SH22 RESPECTIVELY. (VALUES OF 1.E0, -1.E0, OR 0.E0
< /// IMPLIED BY THE VALUE OF SPARAM(1) ARE NOT STORED IN SPARAM.)
---
> /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS
> /// THE SECOND COMPONENT OF THE 2-VECTOR  (SQRT(SD1)*SX1,SQRT(SD2)*>    SY2)**T.
> /// WITH SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// SFLAG=-1.E0     SFLAG=0.E0        SFLAG=1.E0     SFLAG=-2.E0
> ///
> ///   (SH11  SH12)    (1.E0  SH12)    (SH11  1.E0)    (1.E0  0.E0)
> /// H=(          )    (          )    (          )    (          )
> ///   (SH21  SH22),   (SH21  1.E0),   (-1.E0 SH22),   (0.E0  1.E0).
> /// LOCATIONS 2-4 OF SPARAM CONTAIN SH11,SH21,SH12, AND SH22
> /// RESPECTIVELY. (VALUES OF 1.E0, -1.E0, OR 0.E0 IMPLIED BY THE
> /// VALUE OF SPARAM(1) ARE NOT STORED IN SPARAM.)
345,353c348,357
< /// SX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE LX =
< /// (-INCX)*N, AND SIMILARLY FOR SY USING USING LY AND INCY. WITH
< /// SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// SFLAG=-1.E0 SFLAG=0.E0 SFLAG=1.E0 SFLAG=-2.E0
< ///
< ///  (SH11 SH12) (1.E0 SH12) (SH11 1.E0) (1.E0 0.E0) H=() () () () (SH21 SH22),
< /// (SH21 1.E0), (-1.E0 SH22), (0.E0 1.E0). SEE SROTMG FOR A DESCRIPTION OF DATA
< /// STORAGE IN SPARAM.
---
> /// SX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE
> /// LX = (-INCX)*N, AND SIMILARLY FOR SY USING USING LY AND INCY.
> /// WITH SPARAM(1)=SFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// SFLAG=-1.E0     SFLAG=0.E0        SFLAG=1.E0     SFLAG=-2.E0
> ///
> ///   (SH11  SH12)    (1.E0  SH12)    (SH11  1.E0)    (1.E0  0.E0)
> /// H=(          )    (          )    (          )    (          )
> ///   (SH21  SH22),   (SH21  1.E0),   (-1.E0 SH22),   (0.E0  1.E0).
> /// SEE  SROTMG FOR A DESCRIPTION OF DATA STORAGE IN SPARAM.
369,378c373,384
< /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS THE SECOND
< /// COMPONENT OF THE 2-VECTOR (DSQRT(DD1)*DX1,DSQRT(DD2)*> DY2)**T. WITH
< /// DPARAM(1)=DFLAG, H HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// DFLAG=-1.D0 DFLAG=0.D0 DFLAG=1.D0 DFLAG=-2.D0
< ///
< ///  (DH11 DH12) (1.D0 DH12) (DH11 1.D0) (1.D0 0.D0) H=() () () () (DH21 DH22),
< /// (DH21 1.D0), (-1.D0 DH22), (0.D0 1.D0). LOCATIONS 2-4 OF DPARAM CONTAIN
< /// DH11, DH21, DH12, AND DH22 RESPECTIVELY. (VALUES OF 1.D0, -1.D0, OR 0.D0
< /// IMPLIED BY THE VALUE OF DPARAM(1) ARE NOT STORED IN DPARAM.)
---
> /// CONSTRUCT THE MODIFIED GIVENS TRANSFORMATION MATRIX H WHICH ZEROS
> /// THE SECOND COMPONENT OF THE 2-VECTOR  (DSQRT(DD1)*DX1,DSQRT(DD2)*>    DY2)**T.
> /// WITH DPARAM(1)=DFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// DFLAG=-1.D0     DFLAG=0.D0        DFLAG=1.D0     DFLAG=-2.D0
> ///
> ///   (DH11  DH12)    (1.D0  DH12)    (DH11  1.D0)    (1.D0  0.D0)
> /// H=(          )    (          )    (          )    (          )
> ///   (DH21  DH22),   (DH21  1.D0),   (-1.D0 DH22),   (0.D0  1.D0).
> /// LOCATIONS 2-4 OF DPARAM CONTAIN DH11, DH21, DH12, AND DH22
> /// RESPECTIVELY. (VALUES OF 1.D0, -1.D0, OR 0.D0 IMPLIED BY THE
> /// VALUE OF DPARAM(1) ARE NOT STORED IN DPARAM.)
402,410c408,417
< /// DX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE LX =
< /// (-INCX)*N, AND SIMILARLY FOR SY USING LY AND INCY. WITH DPARAM(1)=DFLAG, H
< /// HAS ONE OF THE FOLLOWING FORMS..
< ///
< /// DFLAG=-1.D0 DFLAG=0.D0 DFLAG=1.D0 DFLAG=-2.D0
< ///
< ///  (DH11 DH12) (1.D0 DH12) (DH11 1.D0) (1.D0 0.D0) H=() () () () (DH21 DH22),
< /// (DH21 1.D0), (-1.D0 DH22), (0.D0 1.D0). SEE DROTMG FOR A DESCRIPTION OF DATA
< /// STORAGE IN DPARAM.
---
> /// DX(LX+I*INCX), I = 0 TO N-1, WHERE LX = 1 IF INCX .GE. 0, ELSE
> /// LX = (-INCX)*N, AND SIMILARLY FOR SY USING LY AND INCY.
> /// WITH DPARAM(1)=DFLAG, H HAS ONE OF THE FOLLOWING FORMS..
> ///
> /// DFLAG=-1.D0     DFLAG=0.D0        DFLAG=1.D0     DFLAG=-2.D0
> ///
> ///   (DH11  DH12)    (1.D0  DH12)    (DH11  1.D0)    (1.D0  0.D0)
> /// H=(          )    (          )    (          )    (          )
> ///   (DH21  DH22),   (DH21  1.D0),   (-1.D0 DH22),   (0.D0  1.D0).
> /// SEE DROTMG FOR A DESCRIPTION OF DATA STORAGE IN DPARAM.
```
@IvanUkhov
Copy link
Member

Thank you for the pull request! As far as I understand, you propose not formatting the descriptions of those functions. However, it doesn’t solve the problem. The objective of #21 is to actually come up with an adequate formatting scheme of those edge cases. We’d like to have all the descriptions alike so that the crate looks coherent, well polished. Looking at those ugly descriptions, I’m not sure if it’s possible to come up with an elegant solution. Perhaps the best thing to do is to fix them directly in Netlib’s repository so that the whole community can benefit from this change.

@MartyIX
Copy link
Author

MartyIX commented Feb 1, 2017

Well, my solution is a hack and it is not a nice one, but a user gets correct information. :-) I don't think that one may hope to find a fundamentally better solution without changing external code. I think that it would be certainly for the best to send pull requests to update comments in Netlib's repo to avoid doing heavy lifting in documentation.py as much as possible.

If upstream changes are not a viable path, then maybe the following design would be better:

  1. Initial phase: documentation.py would download source data, would store hashes for each particular source file content and it would parse a nice comment. If a resulting comment is not acceptable, I would manually edit the result. I would save the final comments to be tracked by git.
  2. Update phase: If a source file (or source comment) changes (i.e. if the hash is different), then parse a nice comment again and manually review.

What do you think?

@IvanUkhov
Copy link
Member

IvanUkhov commented Feb 1, 2017

Updating the upstream would be the best. If they don’t want to accept such changes for some reason, we can use your heuristic to bypass (do not output any documentation) those functions so that at least the other functions will get decent descriptions, and that there will be no ugly stuff in the documentation. I’d like to avoid any manual work because it’ll be a burden for the future maintenance.

Regarding those other aspects that you mentioned, I don’t think that the documentation script should do anything else including downloading and hashing anything.

@MartyIX
Copy link
Author

MartyIX commented Feb 1, 2017

(I see a great benefit in tracking what documentation.py returns as comments because then you will know about any external changes but I don't think you agree so I don't want to push it in any way.)

I don't know if there is anything else I can do with this. I don't use blas so I understand the code only superficially. :-)

@IvanUkhov
Copy link
Member

IvanUkhov commented Feb 1, 2017

I absolutely agree with you. It’s really beneficial to keep track of what changes from one version to another. It’s exactly what happens right now in the current workflow. Each time you run c.py and update c.rs, you get to see what’s changed since the last time you performed this operation. First you inspect the changes with git diff, and then you see them in the corresponding commit.

@IvanUkhov IvanUkhov closed this Mar 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants