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

Suppress unit printing and parsing for Dimensionless #12

Closed
averbraeck opened this issue Jul 20, 2023 · 12 comments
Closed

Suppress unit printing and parsing for Dimensionless #12

averbraeck opened this issue Jul 20, 2023 · 12 comments
Assignees
Labels
bug Something isn't working simplification Simplification o code

Comments

@averbraeck
Copy link
Owner

The Dimensionless scalar, vector and matrix print a unit called unit under certain circumstances, and 1 as a unit under other circumstances. unit is not a sensible indication of the unit for a dimensionless number. 1 is not a valid SI-unit. Probably the best solution is to completely suppress the unit printing and parsing for the Dimensionless scalar, vector and matrix.

@averbraeck averbraeck added bug Something isn't working simplification Simplification o code labels Jul 20, 2023
@OTSim
Copy link
Collaborator

OTSim commented Jul 21, 2023

I believe the word "unit" (that gets printed under some circumstances) comes from quantity/Quantity.java line 26.
In a way, this code contradicts the comment on line 24 of that same file.

@averbraeck
Copy link
Owner Author

The problem is that in the current setup, a unit's display string cannot have a length of 0. I have to allow that first before I can see whether all methods of the Dimensionless class such as of(String) still work. I will write some extra unit tests for the Dimensionless class to test whether this is working as intended.

@averbraeck
Copy link
Owner Author

probably, also the SIUnit class needs to be updated that is does not display a unit when the result is dimensionless. I believe it now prints 1 as the unit, which is clearly wrong.

@OTSim
Copy link
Collaborator

OTSim commented Jul 22, 2023

A unit of "1" is not totally bad. But a unit named ""unit"" is wrong. The evaluator (under development as part of djutils-eval) evaluates "PI()" to "3.14159265 unit" but "PI()/PI()" to "1.00000000 1".

@averbraeck
Copy link
Owner Author

A unit's name and id can now be blank, as long as the SI units show that the unit is dimensionless, and the scale is the IdentityScale. In the Unit constructor, this is checked as follows:

Throw.when(
  builder.getId().length() == 0
    && !(builder.getSiPrefixes().equals(SIPrefixes.NONE) 
    && builder.getScale().equals(IdentityScale.SCALE)),
  UnitRuntimeException.class, "Constructing unit %s: id.length cannot be 0", cName);

and

Throw.when(
  builder.getName().length() == 0
    && !(builder.getSiPrefixes().equals(SIPrefixes.NONE) 
    && builder.getScale().equals(IdentityScale.SCALE)),
  UnitRuntimeException.class, "Constructing unit %s.%s: name.length cannot be 0", cName, unitId);

averbraeck added a commit that referenced this issue Jul 23, 2023
This can be done as long as the SI units show that the unit is
dimensionless (SIPrefixes.NONE), and the scale is the IdentityScale
averbraeck added a commit to averbraeck/djunits-generator that referenced this issue Jul 23, 2023
Handled with ##IF - ##ENDIF tags. The Dimensionless of and valueOf
functions relate to averbraeck/djunits#12
averbraeck added a commit that referenced this issue Jul 23, 2023
With new of() and valueOf() functions that have been generated by the
latest version of the djunits-generator project.
@averbraeck
Copy link
Owner Author

The Dimensionless class has been updated and functions as expected. All unit tests pass again.

@averbraeck
Copy link
Owner Author

For the handling of a dimensionless SIScalar, a new issue has been opened as Issue #14.

@averbraeck
Copy link
Owner Author

The Dimensionless and FloatDimensionless classes seem to work as intended. All unit tests pass.

@OTSim
Copy link
Collaborator

OTSim commented Jul 23, 2023

Things are somewhat improved but there are still some inconsistencies. To reproduce the problem run:
System.out.println(new Dimensionless(10, DimensionlessUnit.SI).divide(new Dimensionless(3, DimensionlessUnit.SI)));
System.out.println(((DoubleScalarRel)new Dimensionless(10, DimensionlessUnit.SI)).divide(new Dimensionless(3, DimensionlessUnit.SI)));
This prints
3.33333333
3.33333333 1
N.B.: I propose to add a unit test that checks that the two divide operations in this demonstration code return identical results.

In the first case the division is carried out by Dimensionless.java(391) which simple sets the target unit to DimensionlessUnit.SI
In the second case the division is carried out by DoubleScalarRel.java(100) which does things "the hard way":
1: It calls DoubleScalar.divide to do the real work.
2: DoubleScalar.divide lives in file DoubleScalar.java at line 545. It calls Unit.lookupOrCreateUnitWithSIDimensions to determine the SIUnit of the result. This method construct an SIUnit that has id "1" and name "1". I presume that this is wrong.

By the way: DoubleScalarRel.java(100) is a divide method that returns SIScalar, but the javadoc says that it returns DoubleScalar<?>.

@OTSim OTSim reopened this Jul 23, 2023
@averbraeck
Copy link
Owner Author

Issue #14 has taken care of the "1" that is not printed anymore.

@averbraeck
Copy link
Owner Author

So indeed, Issue #12 was not completely solved and needed Issue #14 to get rid of all the "1" units when the Dimensionless is expressed as a SIUnit scalar. I will close this issue again after all unit tests involving the "1" unit pass as well.

Repository owner deleted a comment from OTSim Jul 23, 2023
@averbraeck
Copy link
Owner Author

Issue #14 is now (almost) completed, which should solve the remaining problems of this issue, so I close it again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working simplification Simplification o code
Projects
None yet
Development

No branches or pull requests

2 participants