-
Notifications
You must be signed in to change notification settings - Fork 169
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
Add date and time utilities #3247
base: master
Are you sure you want to change the base?
Conversation
beutlich
commented
Dec 2, 2019
•
edited
Loading
edited
- Move getTime from System to Internal package and introduce a new package Time with function getTime returning a record, see .Modelica.Utilities.System.getTime() returns bad year #3143 (comment).
- Add leap year utility functions.
- Add date-time to string conversion (using strftime), see Absolute time in Modelica (datetime) ModelicaSpecification#2245.
- Add string to date-time conversion (using strptime (to be added to C-Sources since not available on all systems)), see Absolute time in Modelica (datetime) ModelicaSpecification#2245.
The general idea is good, but:
I tried to look at the Buildings-library. It uses full names in the records, but unfortunately it contains its own leap-year handling (hopefully correct): |
I started with an operator record for TimeType to also allow addition, subtraction etc. That meant, we also would need to consider a TimeDeltaType and conversion to absolute time, which is not straight-forward. Therefore, I rejected this first idea and went for the TimeType record as absolute point in time and without numerical conversion. (String conversion is still to do, but without handling of leap-seconds).
OK, will rename.
Hm, @GallLeo what do you think? @mwetter considers leap years only in range [2020, 2030] which might not be sufficient in a general purpose library. |
If you want to consider leap years, daylight savings, etc... You need to work on some absolute unit of s/etc relative from 1970... I think that's the only sane way of dealing with any kind of delta and ambiguity... Just convert between tm and time_t if you want to use that (and run the calculation in external C). |
Here is a first (and incomplete) draft for conversion of date/time to string. Not sure if it is good enough ❔ . Any feedback is appreciated. |
@HansOlsson @DagBruck @t-sommer @StephanZiegler Yesterday, I noticed by chance that there are Testing.Utilities.Time.{DateTime, Duration} available in Dymola as operator records. This is exactly the way I wanted to go here for MSL, too. I believe these utilities came up as necessary side-product when meaasuring time in Modelica. Now, that I am aware of this library, I am also biased with the further development. My question is, if you could think of contributing or sharing Modelica code for further development as OSS, such that I am sure, no copyright will be violated? |
We would be happy to move the operator records from the Testing library to the MSL. |
b201bac
to
9575f16
Compare
@m-kessler Thank you for the generous offer. I rebased the commits of this PR on master and resolved the merge conficts. This PR branch is now ready to be used. I added you as contributor (with access role = Triage) such that you can directly push to this branch (hopefully). I already created a TimeType record. This can be now enhanced by using the two different operator records from the Testing package. It would be good if we can use the exsting names of the TimeType record components (which are different than the Testing package.) |
9575f16
to
362c49d
Compare
8cc2d55
to
47d29e8
Compare
This requires fromSeconds to be the new default. And it's not possible anymore to create Durations by specifying e.g. hour only. But unless you write tests or doc, this is not really needed I would say.
Creating a Duration does not work anymore that way, due to "e547700 - Make Duration constructors unambiguous"
Co-Authored-By: Thomas Beutlich <[email protected]>
... as the one we had did not catch negative durations with multiple days.
... as this is not supported at the moment.
... to fix inheritance, as its not allowed to extend from an operator record in Modelica 3.4
Conversion for MSL v4.0.0 no longer is suitable, hence migrate to next major version.
f1a3f53
to
059132f
Compare