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

[Feature Request]: timeDelta.totalMicroseconds #26658

Closed
ajpotts opened this issue Feb 5, 2025 · 2 comments · Fixed by #26798
Closed

[Feature Request]: timeDelta.totalMicroseconds #26658

ajpotts opened this issue Feb 5, 2025 · 2 comments · Fixed by #26798

Comments

@ajpotts
Copy link

ajpotts commented Feb 5, 2025

Summary of Feature

Description:
Please add a timeDelta.totalMicroseconds procedure. One application would be for the generation of random seeds.

Is this issue currently blocking your progress?
No, because in arkouda we implemented our own timeDelta.totalMicroseconds procedure. However, it would be more efficient to call it from chapel.

Code Sample

            use Time;

            proc timeDelta.totalMicroseconds(): int{
                return ((days*(24*60*60) + seconds)*1_000_000 + microseconds): int;
            }

            var next: rng.eltType;
            if hasSeed {
                next = rng.next();
            }else{
                const seed =  timeSinceEpoch().totalMicroseconds();
                var randStream0 = new randomStream(rng.eltType, seed);
                next = randStream0.next();
            }
@bradcray
Copy link
Member

bradcray commented Feb 5, 2025

Thanks for filing this, @ajpotts ! Would you be interested in filling a PR contributing this routine to the Time module?

@lydia-duncan lydia-duncan self-assigned this Feb 18, 2025
lydia-duncan added a commit that referenced this issue Mar 4, 2025
…6798)

[reviewed by @DanilaFe and @bradcray]

Resolves #26658 

Note that the implementation for totalMicroseconds is strongly based on
the suggestion from ajpotts, so credit to it should go to her and the
Arkouda team.

Updated some tests and a module that were converting the result of
totalSeconds to microseconds so that they use the new method instead.

Added a test of the new methods.

Passed a standard paratest with futures and a performance run of
`test/studies/ssca2` looked to be at the same granularity for the units
printed out.
@lydia-duncan
Copy link
Member

Hi @ajpotts - I just merged #26798, which adds support for this and totalMilliseconds(). Thanks for the feature request and let me know if there's anything else along these lines you'd be interested in seeing (I opened #26820 for totalNanoseconds but that will be a more involved adjustment, since we don't store nanoseconds today)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants