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

Lines.RMatrix and Lines.XMatrix scale input before setting #36

Open
kdheepak opened this issue Feb 22, 2019 · 5 comments
Open

Lines.RMatrix and Lines.XMatrix scale input before setting #36

kdheepak opened this issue Feb 22, 2019 · 5 comments

Comments

@kdheepak
Copy link
Member

@PMeira I noticed that RMatrix and XMatrix functions scale the input when setting values

julia> Lines.RMatrix()
2×2 Array{Float64,2}:
 0.000409951  0.000118095
 0.000118095  0.000409951

julia> Lines.RMatrix(Lines.RMatrix())

julia> Lines.RMatrix()
2×2 Array{Float64,2}:
 4.09951e-7  1.18095e-7
 1.18095e-7  4.09951e-7

julia> Lines.RMatrix(Lines.RMatrix())

julia> Lines.RMatrix()
2×2 Array{Float64,2}:
 4.09951e-10  1.18095e-10
 1.18095e-10  4.09951e-10

julia> Lines.Units()
LineUnits_ft(5)

What is the reason behind this behavior?

@PMeira
Copy link
Member

PMeira commented Feb 22, 2019

I don't know, I posted something related on the forums and got no comments. I also did search the forums but couldn't find anything near the date of the commit. SourceForge's search is not great though, I could have missed something.

https://sourceforge.net/p/electricdss/discussion/experts/thread/459afcb4/

It was changed in 2017 and it's inconsistent for those two properties (R1 and X1) even today in the official distribution -- dss-extensions/electricdss-src@c081ae2

Before that, there was no relevant changes in the Lines interface code. After, there was a fix in Lines_Get_Cmatrix in 2018.

@PMeira
Copy link
Member

PMeira commented Feb 22, 2019

Oh, and to clarify, they scale in the getter, not in the setter.

@kdheepak
Copy link
Member Author

kdheepak commented Feb 22, 2019

I see.

New Linecode.4/0Triplex nphases=2 units=kft      !ohms per 1000 ft
~ rmatrix=[  0.40995115   0.11809509 |  0.11809509   0.40995115 ]
~ xmatrix=[  0.16681819   0.12759250 |  0.12759250   0.16681819 ]
~ cmatrix=[  3.00000000  -2.40000000 | -2.40000000   3.00000000 ]
~ Normamps=156  {156 1.25 *}

This is the linecode that is attached to that specific Line. And the units is kft. But internally, it is probably stored as two things, the scaling factor and actual unit itself. This is why the Units() function returns ft.

julia> Lines.Units()
LineUnits_ft(5)

And the conversion is correctly applied to return the value with the correct units in the getter. However, in the setter we assume that the unit is what is defined original line code.

I think it makes sense to multiply the UnitsConvert to the value from the setter as well. What do you think @PMeira?

I think this because a user is going to use the Units() function to get 5 or LineUnits_ft and assume that the setter accepts values in that unit. There's no way through the interface to get the scaling factor, right?

@PMeira
Copy link
Member

PMeira commented Feb 23, 2019

I think it makes sense to multiply the UnitsConvert to the value from the setter as well. What do you think @PMeira?

@kdheepak Something like that (there seems to be some missing side-effects besides the unit convertion), as we usually expect that a call like Lines.RMatrix(Lines.RMatrix()) would not change the matrix, and the current version doesn't respect that.

The COM PDF docs don't mention anything about units, but the Direct DLL mentions the same thing from the COM helpstrings that we have imported:

Lines.RMatrix read
This parameter gets the resistance matrix (full), ohms per unit length. (...)

Lines.RMatrix write
This parameter sets the resistance matrix (full), ohms per unit length. (...)

So it seems it was intended to use the same units, the missing factor on write is probably an oversight.

I'll review the related code better this weekend. I think there are also issues with the capacitance values/matrices, maybe both in the parser side and in the DLL API side. I also noticed that in the parser there are some conditions that I didn't see in the API code. For reading:

       // From Line.pas
        12:
            for i := 1 to FNconds do   // R matrix
            begin
                for j := 1 to i do
                begin  // report in per unit Length in length units
                    if GeometrySpecified or SpacingSpecified then
                        Result := Result + Format('%-.7g', [Z.GetElement(i, j).re / len]) + ' '
                    else
                        Result := Result + Format('%-.7g', [Z.GetElement(i, j).re / FUnitsConvert]) + ' ';
                end;
                if i < FNconds then
                    Result := Result + '|';
            end;

For writing, there is no extra unit handling, but it changes other things:

                12..14:
                begin
                    FLineCodeSpecified := FALSE;
                    SymComponentsModel := FALSE;
                    ResetLengthUnits;
                    KillGeometrySpecified;
                    KillSpacingSpecified;
                end;

Looks like we really need that better handling of side-effects.

I spent a couple minutes searching the repositories that use OpenDSS here on GitHub and on SourceForge. I didn't find any that reads or writes (R/X)Matrix through the API, only saw code that uses the parser. Maybe some people found this bug/strange behavior and just used the parser instead. Either way, it doesn't look like we would break too much code.

There's no way through the interface to get the scaling factor, right?

No, not directly. I think it's a good idea to expose some of the utility functions used for this kind of thing (e.g. ConvertLineUnits, GetUnitsCode).

@kdheepak
Copy link
Member Author

Yes, I don't expect people are changing the RMatrix of a line / linecode using the setter functionality. We could even request a call with EPRI folks and see if they can help us answer questions and sort out any issues we have.

Thanks again for looking into this! If you want to open issues and assign them to me, I'm willing to get my hands dirty as well. I can submit PRs here for the necessary changes. I feel a little bad just opening all these issues and making you look into them ;)

@PMeira PMeira added the lines label Aug 7, 2020
@PMeira PMeira added this to the 1.0 milestone May 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants