nikhilgupta10
commented
7 years ago nikhilgupta10 imported these comments from Sourceforge: "andyfisher": * attachment _testfuse.glm added ,
"dchassin":Why aren't these functions being done earlier? Commit should just be clearing internal states that are no longer needed. It should be changing anything that is required to verify consistency between objects.
There should be no need to commit by rank. If there appears to be a need it probably because something isn't be done earlier when it should.
,
"jcfuller":Negative. There are reasons for performing some things in commit, mainly for speed reasons, on secondary calculations. This is mainly due to the fact that some calculations really only need to be performed once (i.e., zeroing out voltage in FBS when the phase of the node is disconnected, or calculating power on each link object), however, there is no way to \know\ that the sync cycle is over. So, there are some \clean-up\ things that have a logical place in commit.
Unfortunately, something in v3.0 has changed the order in which the commit function cycles through the objects (tests that pass in v2.3 don't in v3.0 because the order of commit operation has changed). This change is a way to guarantee the order of operation, rather than on the whims of the operating system or the GLM load order.
,
"dchassin":Understood. The change in V3 is probably due to support for threading.
I still contend that commits should not be engaged in making \meaningful\ changes such as those that assert would be checking because it raises the question of whether another iteration would be required. Also zeroing out values is not intrinsically time-consuming but I can certainly see case where one-time sync activities could be. This was in fact the purpose for post-sync passes. I assume your point is that we're trying to avoid a post-sync pass where none would be required otherwise. Sounds to me a little like were putting speed concerns ahead of consistency and clarity. We should certainly proceed with caution because this doesn't make a lot of sense to me and is potentially opening the door to the dreaded nemesis called complexification.
,
"jcfuller": * cc jcfuller removed
- owner changed from mhauer to dchassin
- status changed from new to assigned
- priority changed from major to critical
Dave will write specifications for commit, pre-commit, and \observer status. We'll then discuss from there.
,
"dchassin": * status changed from assigned to accepted ,
"dchassin": * cc ftuffner added
Proposed modification of commit behavior in https://sourceforge.net/apps/mediawiki/gridlab-d/index.php?title=How_To_Write_GridLAB-D_Modules#Commit.
,
"dchassin":Started work with r3952.
,
"dchassin": * owner changed from dchassin to andyfisher
- status changed from accepted to assigned
The fix posted in r3953 addresses the 5 taxonomy feeder errors (for single-threaded runs only). However, it changes the error in the powerflow/autotest/test_fuse.glm, as follows:
WARNING [INIT] : gridlabd.conf was not found
WARNING [INIT] : Fuses only work for the attached node in the FBS solver, not any deeper.
WARNING [2000-01-01 12:00:00 PST] : Phase C of fuse:12 (fuse_test) just blew
ERROR [2000-01-01 12:00:00 PST] : Assert failed on fuse_test: phase_C_status=0 did not match 1
ERROR [2000-01-01 12:00:00 PST] : model commit failed
FATAL [2000-01-01 12:00:00 PST] : shutdown after simulation stopped prematurely
dump to 'gridlabd.xml' complete
FATAL [2000-01-01 12:00:00 PST] : environment startup failed: No such file or directorySince I don't know what the test is supposed to do, I can't be sure whether I fixed it or made it worse. Before I finish implementing this change, I'd like confirmation that this is caused by something in this fix to commit.
To fix this for multi-threaded runs, we need to split the commit into two separate MTI lists so that the second list waits for the first to complete before starting.
,
"andyfisher": * owner changed from andyfisher to dchassin
I accidentally changed the player file for phase C when trying to figure out why test_fuse was failing in ticket 684 and not in v2.3. I also added a stoptime to the test_fuse so that the simulation stops when the player files end. The test passes. This test passing along with the 5 other taxonomy_feeders confirmed that this commit order did fix the issue. Now all that needs to be done is to get this to work multi-threaded.
,
"dchassin": * status changed from assigned to accepted ,
"dchassin": * owner changed from dchassin to andyfisher
- status changed from accepted to assigned
Multithread code posted in r3952. Ready for final validation before merging into trunk.
,
"dchassin":The enhancements related to follow-up on this ticket are tracked in #722.
,
"jcfuller": * type changed from enhancement to validation ,
"andyfisher": * status changed from assigned to closed
- resolution set to fixed
Fixes were committed to trunk in r3970
,
There are a number of autotests that are failing due to the assert object asserting a value before that value has been propertly updated. These all appear to be happening because the function to update the value happens in commit. These functions include fuse updating its status (blown/good), node zeroing out voltages on phases it doesn't have (i.e. Va and Vb = 0 with phases = CN). These functions happen in commit and commit has no rank order to verify whether the assert object's parent has run its commit functions. I've verified that the assert values and the actual value match up when removing the assert.
,