Dynamic Time History Collection #1360

wrtobin · 2021-03-17T18:31:35Z

Fix [Bug] History collection give out of bound error with EmbeddedSurfaceSubRegion. #1356
Fix [Bug] Time history in parallel: abnormally slow setup and crash on exit #1309
Resolve [Feature] Time History output including spatial coordinates #1203
Collect data from arrays with an index set that can change size on any MPI rank during execution
Account for the changing sizes in buffered collected data when writing to file
- Using the minimum index count from a rank involved in the writing works. Any larger and the HDF5 mpio routines error. Minimally this means doing writes with chunk size 1, since any ranks with 0 indices are excluded from the subcomm used to write a specific history 'row'. Efficiency of this approach is questionable, but this is the largest the chunk can reliably be.
Unit testing for (1) expanding and contracting data collection/output from the same process (2) repeated file writing with (1)
Allow collection of anything in the data repository (that is able to be collected).
- Since the dynamic collection changes the extent of the serialized portion of the time history 'row' that each MPI rank writes, we should also be able to use this to collect indexing data as well in order to help track specific items in a time history over the course of the simulation.

…needs testing

castelletto1 · 2021-04-22T17:19:22Z

Hi @wrtobin, are you planning on taking care of #1203 within this PR?

wrtobin · 2021-04-22T17:29:17Z

@castelletto1
I haven't added that yet, but it should be doable.

This PR DOES include the ability to collect anything in the dataRepository (that is packable), which includes objects that aren't necessarily associated with the mesh (linear algebra package information, etc). So I'll have to add some internal logical to only add the coordinate information when collecting from mesh objects.

I've added it to the checklist.

…tically when collecting from object managers / fields, account for dynamic set changes for the coordinates

…anging file row prealloc to 1 so that coords being written and never updated don't have erroneous zero rows

wrtobin · 2021-05-03T16:34:38Z

This requires a rebaseline as the tests deriving from sedov_base.xml will restart fail due to schema changes.

…utput, also fixing an init collection size 0 bug

…cally output when collecting from mesh objects

…ible

corbett5 · 2021-05-20T16:45:24Z

src/coreComponents/fileIO/timeHistory/PackCollection.cpp

-  HistoryCollection::initializePostSubGroups();
+  if( !m_initialized )
+  {
+    DomainPartition & domain = this->getGroupByPath< DomainPartition >( "/Problem/domain" );


@rrsettgast is this preferred over getGlobalState().problemManager().domain()?

I would think to prefer getGlobalState().problemManager().domain(), which is what I was using originally, this came in via a merge and I left it.

It might be the way it is to avoid dependencies.

We have to fix this still...I was avoiding calls to getGlobalState() due to dependency issues, but I hadn't provided an accessor for domain. It isn't pleasant as it stands.

CusiniM

This is great! I would add an integrated test in which some non mesh objects are collected (e.g., nonlinear and linear iterations) so that we have a working example in there.

CusiniM · 2021-05-27T17:30:21Z

src/coreComponents/physicsSolvers/solidMechanics/benchmarks/Sneddon-Validation.xml

-      objectPath="ElementRegions/Fracture/embeddedSurfaceSubRegion"
-      fieldName="elementCenter"
-      minSetSize="120"/>      
+      fieldName="displacementJump" />
  </Tasks>


no need for a min set size coz now the collection just gets resized every time the field changes size?

Yes exactly.

There can still be trailing 0s though since the HDF arrays aren't ragged / AoAs. Practically now we just expand the dataset as needed, which will add 0s to all previous 'rows', and we never contract the dataset: it will be sized to accommodate the largest set of collected data from the run.

How hard would it be to write it out AoA style?

IIRC it will require defining a couple data types, one for the values extracted by a single index, and another variable-length type using the first type. Not sure how it plays in a dataspace with an unlimited dimension.

The bigger unknown is dealing with the write in parallel w/ MPIO since HDF5 is a bit cumbersome in that respect... it might handle variable-length writes from processes transparently, or might require explicitly being told everything about the write (which is mostly the case right now).

I guess this will only happen if the collection has not been written to file yet. So, basically, if the field is resized in the same packCollection event before being written to file then all time-steps will have the field with the maximum size. But this should not happen if an output event occurs before the resizing. Am I wrong?

A size increase at any point results in the dimensions of the hdf dataspace being changed, so even rows already written to file will have 0s appended when the extent is changed.

Correlating the rows and the mesh object coordinates should in most cases allow the determination of which elements are padding elements and which are not.

rrsettgast

just some nitpick things

src/coreComponents/dataRepository/HistoryDataSpec.hpp

src/coreComponents/fieldSpecification/FieldSpecificationManager.hpp

src/coreComponents/fileIO/Outputs/TimeHistoryOutput.cpp

src/coreComponents/fileIO/timeHistory/HistoryIO.hpp

rrsettgast · 2021-06-04T19:52:16Z

src/coreComponents/fileIO/timeHistory/PackCollection.cpp

-  HistoryCollection::initializePostSubGroups();
+  if( !m_initialized )
+  {
+    DomainPartition & domain = this->getGroupByPath< DomainPartition >( "/Problem/domain" );


We have to fix this still...I was avoiding calls to getGlobalState() due to dependency issues, but I hadn't provided an accessor for domain. It isn't pleasant as it stands.

src/coreComponents/fileIO/timeHistory/PackCollection.cpp

Co-authored-by: Randolph Settgast <settgast1@llnl.gov>

castelletto1 · 2021-06-19T01:17:35Z

I tried the integrated test PoroElasticTerzaghi_FIM.xml. Inspecting the pressure history output I see:

I understand the pressure dataset: each row gives the pressure at an output time level (as shown in the pressure Time dataset) for the cell collection. What is not clear to me is the pressure elementCenter dataset: I would expect in each row the cell 0-coordinates -- in this snapshot, 1- and 2-coordinates look the same -- at the desired output time level, but I can only see zero values.

Note that all values in the pressure dataset are zero because the traction field specification has to be done differently. This has been fixed in #1401 . However, this should have no impact on the pressure elementCenter dataset.

wrtobin · 2021-06-21T18:01:47Z

@castelletto1
I made a change to when the coordinates were first collected at some point which removed an extra row from the output (in order to get direct correspondence between the "time index" in all output datasets). Apparently without that extra collection the coordinates of static sets wouldn't be correctly collected due to this bug.

The last commit should fix this bug, thanks for catching it.

wrtobin · 2021-07-01T16:35:04Z

This should be ready to go in.

https://github.com/GEOSX/integratedTests/pull/139 needs to be merged in first to account for the XML changes to the baselines that use the time history functionality.

wrtobin added 6 commits March 16, 2021 10:47

avoid invalid mem access if init size is zero

f3cb4d4

working on allowing dynamic resizing of time history data collected, …

4c1e661

…needs testing

explicitly update the index sets...

904e6c3

flip size change flag back to false after a write operation

4ca8932

clarifying comment

c1b5a0f

comment clarification

c89b268

wrtobin added effort: 1 week type: cleanup / refactor Non-functional change (NFC) type: feature New feature or request labels Mar 18, 2021

debugging based on unit test failure

bafda2b

wrtobin removed the new label Mar 25, 2021

wrtobin added 7 commits April 27, 2021 15:06

collect any packable item in the datarepo, collect coordinates automa…

4146696

…tically when collecting from object managers / fields, account for dynamic set changes for the coordinates

debugging and eliminating parallel and other usage bugs

7574b73

index set bugfixes

09ebc8a

removing serial file writer since it is now functionally obsolete, ch…

e4bf08b

…anging file row prealloc to 1 so that coords being written and never updated don't have erroneous zero rows

rst and release build tweaks

ed81512

doxy and uncrustify

a2f983e

tutorial debugging

d605f54

wrtobin marked this pull request as ready for review May 3, 2021 16:33

wrtobin added flag: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests labels May 3, 2021

wrtobin added 4 commits May 17, 2021 11:14

Merge branch 'develop' of github.com:GEOSX/GEOSX into develop

0435ba3

merging from dev

e06f5a7

gcc-8 excessive capturing or bad scope resolution

50a6edb

remove old xml spec

1cc8395

wrtobin added 3 commits May 19, 2021 12:52

updating sneddon validation benchmark/tutorial for new time history o…

efd32fe

…utput, also fixing an init collection size 0 bug

removing existing explicit coordinate collection since it is automati…

777c1e1

…cally output when collecting from mesh objects

make multiple writers/collectors operating on the same file more feas…

7e43ef9

…ible

wrtobin requested review from CusiniM, corbett5, francoishamon, castelletto1 and rrsettgast May 20, 2021 15:57

corbett5 reviewed May 20, 2021

View reviewed changes

CusiniM approved these changes May 27, 2021

View reviewed changes

wrtobin mentioned this pull request Jun 3, 2021

[Bug] HDF5-DIAG: Error detected in HDF5 (1.10.5) #1427

Closed

rrsettgast approved these changes Jun 4, 2021

View reviewed changes

wrtobin and others added 2 commits June 17, 2021 09:49

Apply suggestions from code review

a43ae06

Co-authored-by: Randolph Settgast <settgast1@llnl.gov>

pr review tweaks, couple xml changes

c4d2572

coord collection bugfix

6b9aae5

wrtobin and others added 2 commits June 22, 2021 11:44

Merge branch 'develop' into feature/wrtobin/dynamic-time-hist

33659dd

Updating tutorial 8

0a4f84d

castelletto1 approved these changes Jun 25, 2021

View reviewed changes

wrtobin added 2 commits July 14, 2021 12:20

Merge branch 'develop' into feature/wrtobin/dynamic-time-hist

96f5ead

update integratedTests after merge

4bd2428

wrtobin added ci: run CUDA builds Allows to triggers (costly) CUDA jobs and removed effort: 1 week flag: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests labels Jul 15, 2021

rrsettgast merged commit d1e9ef7 into develop Jul 15, 2021

rrsettgast deleted the feature/wrtobin/dynamic-time-hist branch July 15, 2021 16:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Time History Collection #1360

Dynamic Time History Collection #1360

wrtobin commented Mar 17, 2021 •

edited by rrsettgast

Loading

castelletto1 commented Apr 22, 2021

wrtobin commented Apr 22, 2021 •

edited

Loading

wrtobin commented May 3, 2021

corbett5 May 20, 2021

wrtobin May 20, 2021

corbett5 May 20, 2021

rrsettgast Jun 4, 2021

CusiniM left a comment

CusiniM May 27, 2021

wrtobin May 27, 2021

corbett5 May 27, 2021

wrtobin May 27, 2021

CusiniM May 27, 2021

wrtobin May 27, 2021

rrsettgast left a comment

rrsettgast Jun 4, 2021

castelletto1 commented Jun 19, 2021

wrtobin commented Jun 21, 2021 •

edited

Loading

wrtobin commented Jul 1, 2021

Dynamic Time History Collection #1360

Dynamic Time History Collection #1360

Conversation

wrtobin commented Mar 17, 2021 • edited by rrsettgast Loading

castelletto1 commented Apr 22, 2021

wrtobin commented Apr 22, 2021 • edited Loading

wrtobin commented May 3, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CusiniM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rrsettgast left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

castelletto1 commented Jun 19, 2021

wrtobin commented Jun 21, 2021 • edited Loading

wrtobin commented Jul 1, 2021

wrtobin commented Mar 17, 2021 •

edited by rrsettgast

Loading

wrtobin commented Apr 22, 2021 •

edited

Loading

wrtobin commented Jun 21, 2021 •

edited

Loading