Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Allow Formulas in Coefficients File - #332

Closed
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks
Closed

Allow Formulas in Coefficients File#332
danielsclint wants to merge 8 commits into
ActivitySim:developfrom
danielsclint:ft_error_checks

Conversation

@danielsclint

@danielsclintdanielsclint commented Aug 25, 2020

Copy link
Copy Markdown

Adds twothree four components and cleans up one error in the test data.

  1. Allow formulas in the coefficients files. This was previously allowed and it makes defining coefficients for some models more succinct. In the ARC case, several mode choice coefficients are multiples of IVT. Specifying the formula in the coefficient files makes it clear of the relationship in the model configuration.

  2. try...except blocks to help track down which file is causing the error. Some models have underlying connections to other models (e.g., mode_choice), so its not always apparent which model the error is originating. This adds a simple output to more clearly delineate which user input file is causing problems.

image

  1. [Update 8/27/2020] Move up coefficient processing in vectorize_tour_scheduling.py to ensure the coefficients are available to the preprocessor (Coefficients by Segment Necessary for Preprocessor #333)

  2. [Update 8/27/2020] Point joint_tour_destination.py to use the joint_tour_destination.yaml file. (Joint Tour Destination Changed to Use Non-Mandatory Destination Choice Model Settings #335)

  3. 4.3. Remove duplicate coefficient specification in non_mandatory_tour_destination_coeffs.csv.

Review Criteria Responses

  1. Does it contain all the required elements, including a runnable example, documentation, and tests?
    This code doesn't change the examples or documentation. It is a background fix to restore prior functionality.

  2. Does it implement good methods (i.e. is it consistent with good practices in travel modeling)?
    Yes. Better debugging of user files.

  3. Are the runtimes reasonable and does it provide documentation justifying this claim?
    This change has no material impact on runtimes.

  4. Does it include non-Python code, such as C/C++? If so, does it compile on any OS and are compilation instructions included?
    No. This is a Python-only change.

  5. Is it licensed with the ActivitySim license that allows the code to be freely distributed and modified and includes attribution so that the ‘provenance’ of the code can be tracked? Does it include an official release of ownership from the funding agency if applicable?
    This work was done under contract to ARC, and, presumably, ARC is providing the changes without any additional licensing beyond the existing ActivitySim licensing.

  6. Does it appropriately interact with the data pipeline (i.e. it doesn't create new ways of managing data)?
    This change does not impact the data pipeline.

  7. Does it include regression tests to enable checking that consistent results will be returned when updates are made to the framework?
    No regression testing has been done.

  8. Does it include sufficient test coverage and test data for existing and proposed features?
    No

  9. Any other comments or suggestions for improving the developer experience?
    This PR restore some functionality that was lost during PR major work on phase 5 #325.

@coveralls

coveralls commented Aug 25, 2020

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.007%) to 81.083% when pulling 53d03f8 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 81.049% when pulling c04fe90 on danielsclint:ft_error_checks into 05a25f6 on ActivitySim:develop.

@danielsclintdanielsclint changed the title Error check for COEFFICIENT filesAllow Formulas in Coefficients FileAug 26, 2020
@bstabler

Copy link
Copy Markdown
Contributor

thanks @danielsclint. @toliwaga is going to setup a meeting to discuss design considerations around supporting formulas in coefficient files and estimation integration.

@danielsclint

Copy link
Copy Markdown
Author

Based on conversation, closing this PR and will issue a modified PR without the change to constants in the coefficient file.

@danielsclint
danielsclint deleted the ft_error_checks branch October 23, 2020 04:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danielsclint@coveralls@bstabler