Major Wallet locking Issue with NativeAssets overflow in UTXOs

Essential Information
Daedalus 3.3.0#16328 Frontend
Public-Testnet
Steps to Reproduce
Wallet-Owner:

Create a new Wallet in Daedalus/Cardano-Wallet via SeedPhraseMethod
Sends 10M Ada to fund the Wallet to the first Receive-Address

Now a bad actor starts to come in:

Sends<PHONE_NUMBER>3709551614 XCoins to the first Receive-Address
Sends again<PHONE_NUMBER>3709551614 XCoins to the first Receive-Address
Sends again<PHONE_NUMBER>3709551614 XCoins to the first Receive-Address

Now the wallet looks like this:

Wallet-Owner:

Sees the 6 additional ADA but yes, they are just there, Wallet-Owner is happy about free ADA
Wallet-Owner wants to send 8M ADA out of the Wallet
Transaction fails

Why does it fail? Because cardano-wallet is also using two of the additional UTXOs that the bad actor sent over. It tries to consolidate the Native Assets of course, but the sum of two of the UTXOs generates an overflow. Because the maximum possible value is<PHONE_NUMBER>3709551614 or<PHONE_NUMBER>3709551615, doesn't matter.
The Wallet is now locked up.
You can maybe send out smaller pieces with the hope that the auto UTXO picking algo is not using any of the asset UTXOs again.
You also can't get rid of the assets, because first - Daedalus does not support it right now, but second - You will also end up with more than the maximum amount in the transaction and the node will fail.
Resolution
We need a much better UTXO selection algo ASAP! Or even an advanced mode in the wallets to hand-pick the UTXOs you wanna use. If this happens on a cli based account like for SPOs, this is solvable. But a normal User can't solve this. Funds are locked up, or kind of. But imagin the reaction of the Wallet-Owner. Also, restoring the wallet on another Frontend like Yoroi or Adalite results in the same error.
I can provide the DevTeam a SeedPhrase if someone wants to restore the TestWallet i created.
Jira Issue number
ADP-726

I have also independently verified this in my Daedalus Testnet 3.3.0 wallet using a wallet with utxo's with sub-max tokens as well as several max-token utxo's and a few non-token utxo's. The amount that can be sent either to your own wallet or to an external wallet randomly fails depending upon which utxo's Daedalus selects for the tx-in. In a wallet with ~28 Ada total in it, 20 non-tokened and 8 tokened, the first value I was able to send successfully was ~14 Ada, then the maximum I could send decreased to 8 Ada, then 4 Ada, then finally 1 Ada per send operation.

@gitmachtl Thanks for reporting this. I was actually discussing this very limitation with the team this morning. The underlying issue is not an overflow, but actually the opposite. The wallet does handle the high token quantity just fine...  but does it a bit too much. As a matter of fact, the serialization codecs do expect asset's quantities to fit in a uint64. Yet, in case you have assets with very large quantity (i.e. > 2^64), the wallet may end up creating a change output that is too large and can't be serialized and eventually, fails to be submitted.
There's some remaining work to be done on the creation of change outputs. In the meantime, an immediate work-around is to "manually" send out extra money to yourself, forcing the wallet to create those "small enough" change outputs. This is however not acceptable as a long-term solution.
We're on it :)

We need a much better UTXO selection algo ASAP!

Please don't be too harsh on months of efforts :). Remember that we are the first blockchain ever to try to tackle this problem natively. It's something whic has been developed with great care and the UTxO selection algorithm is the result of good engineering. Yet, there are many aspects to it and it spans over multiple components and as always, it's the integration of end components which is the most painful.

Please don't be too harsh on months of efforts :)

I am on your side, thats why i am testing such stuff on the edge cases. We all care much about cardano, so also about possible backlashes from 3rd parties about such issues. Its no problem to sort theses things out on the CLI, but its not possible for the "normal" cardano user using the major wallets. So maybe we need an "expert" mode to make handpicking of input utxos possible in the future.

There's no cause for alarm here. There's no overflows here and no risk to anyone's funds.
Yes the wallet will need a few more tweaks to the coin selection to account for the new things we can do with multi-assets. The wallet team are aware of that. They've been updating the wallet in phases, starting with basic support for multi-asset.

A possible solution is to detect larger --tx-out numbers than<PHONE_NUMBER>3709551615 and automatically split them into multiple --tx-outs to the same destination address.

Indeed. I'm sure the solution will be along those lines.

@gitmachtl wrote:

A possible solution is to detect larger --tx-out numbers than<PHONE_NUMBER>3709551615 and automatically split them into multiple --tx-outs to the same destination address.

@dcoutts wrote:

Indeed. I'm sure the solution will be along those lines.

That is indeed the solution we've chosen to go for. (See https://github.com/input-output-hk/cardano-wallet/pull/2536)
When we construct a change output, if we detect that any quantity would exceed the maximum, we partition the output into n outputs whose bundles are approximately equal in size. (We cannot guarantee exact equality, as sometimes a token quantity may not be exactly divisible by the number of parts in which we'd like to divide it.)
In order to determine the number of parts in which to split a given bundle, we choose the smallest number of parts that still allows us satisfy the goal of not exceeding the maximum allowable quantity in any given bundle.

The solution is not yet implemented in the Flight Version 4.0.2-FC3#17097 i guess?

Hi @gitmachtl, Daedalus 4.0.2-FC2 is using cardano-wallet v2021-03-04, so it should include the fix for this particular issue.
When it says "An error occurred," the actual error from cardano-wallet is probably a different one to before. The tx may now be rejected for a new reason. @jonathanknowles will have more details.

Hi @gitmachtl, Can you share what is the error that cardano-wallet throws when you hit "Send"?
Errors should be displayed in Daedalus developer console. To open it click (Ctrl + Alt + i) and go to console tab.
Once you click "Send" the actual error that is coming from cardano-wallet backend should be displayed there.

Can you share what is the error that cardano-wallet throws when you hit "Send"?

It is a Ledger HW-Wallet with Ledger Cardano App 2.2.0.
Here is the console output. Verification on the Ledger is working normal.
(anonymous) @ renderer.js:34
emit @ events.js:210
onMessage @ init.ts:50
renderer.js:34 [2021-03-19T18:16:25.605Z] [daedalus.*testnet*:error:ipcRenderer] {
  "msg": "AdaApi::createExternalTransaction error",
  "data": {
    "error": {
      "code": "malformed_tx_payload",
      "message": "I couldn't verify that the payload has the correct binary format. Therefore I couldn't send it to the node. Please check the format and try again."
    }
  }
}
(anonymous) @ renderer.js:34
emit @ events.js:210
onMessage @ init.ts:50
asyncToGenerator.js:6 Uncaught (in promise) ApiErrorÂ {isFinalError: false, id: "api.errors.GenericApiError", defaultMessage: "!!!An error occurred.", values: {â€¦}, code: "malformed_tx_payload",Â â€¦}tempError: (...)clause: (...)forceSet: (...)additionalValues: (...)isFinalError: falseid: "api.errors.GenericApiError"defaultMessage: "!!!An error occurred."values: {code: "malformed_tx_payload", message: "I couldn't verify that the payload has the correctâ€¦ the node.  It seems to be different issue related with HW wallets support. I've managed to reproduce it as well and we will look into it.


Federated logout test for OIDCIdentityFederationTestCase

Hi @Chamodii, Thank you for your contributions. These tests will be added with some modifications from this pull request. Hence we are closing this PR.


JoinTransitionDefinition re-initialisation of late property

Given the following state machine:
StateMachine.create((g) => g
      ..initialState<Idle>()
      ..state<Idle>((b) => b..on<OnFocus, Typing>())
      ..state<Typing>((b) => b
        ..on<OnBlur, Idle>()
        ..onFork<OnValueChange>(
            (b) => b..target<Point>()..target<Autocomplete>(),
            condition: (e) => e.isFormula)
        ..coregion<TypingFormula>((b) => b
          ..onJoin<OnValueChange, Typing>(condition: (e) => !e.isFormula)

          // Autocomplete state machine
          ..state<Autocomplete>((b) => b)
          // Point mode state-mahcine
          ..state<Point>((b) => b)))
      ..onTransition((from, e, to) => print(
          'Received Event $e in State ${from!.stateType} transitioning to State ${to!.stateType}')));

class OnValueChange implements Event {
  final bool isFormula;
  const OnValueChange({required this.isFormula});
}

When running the following unit test:
test('First Test', () async {
      var machine = MyMachine().machine;

      expect(machine.isInState<Idle>(), isTrue);

      machine.applyEvent(OnFocus());
      machine.applyEvent(OnValueChange(isFormula: true));
      await machine.waitUntilQuiescent;

      expect(machine.isInState<TypingFormula>(), isTrue);

      machine.applyEvent(OnValueChange(isFormula: false));
      await machine.waitUntilQuiescent;

      expect(machine.isInState<Typing>(), isTrue);
      expect(machine.isInState<TypingFormula>(), isFalse);
    });

I get the following error:
LateInitializationError: Field '_triggeredBy@70402403' has already been initialized.

Which seems to be related with JoinTransitionDefinition and late final E _triggeredBy;.
Not sure if this is an actual issue, or a problem with the way I have setup my machine.

I have a fix for this and will release shortly.

Fixed in 2.0.4.


Async_hooks

"postinstall": "node_modules/.bin/rn-nodeify --install async_hooks,net,vm,fs,url,tls,querystring,zlib,stream,path,crypto,events,process,util,constants,http  --hack"
},
async_hooks doesnt seem to work. also stream/web. Any idea how to get these to work?

I have this issue too

you'd need to first find or develop a shim for async_hooks, does one exist?
what's stream/web?


Update frontend for new Usul contract

Overview
The frontend needs to be updated to support the new modified Usul contract.
Changes affecting the frontend include:
LinearTokenVoting

OZLinearVoting contract renamed to LinearTokenVoting
receiveStrategy function renamed to timelockProposal
New isProposer function that returns whether an address can submit a proposal. This probably won't need to be implemented on the FE yet since this implementation always returns True
getProposal function added for getting proposal details
calculateWeight renamed to getVotingWeight
quorumDenominator constant updated from 100 to 1,000,000

FractalUsul

submitProposalWithMetadata function renamed to submitProposal
submitProposal interface changed to:
function submitProposal(       address strategy,       bytes memory data,       FractalUsul.Transaction[] calldata transactions,       string calldata metadata   ) external;
ProposalCreated event changed to:
    event ProposalCreated(       address strategy,       uint256 proposalId,       address proposer,       Transaction[] transactions,       string metadata   );
This function now emits a single event, called ProposalCreated. This event had a duplicate of proposald / proposalNumber, so this duplicate was also removed. This event now includes the following fields:
    event ProposalCreated(       address strategy,       uint256 proposalId,       address proposer,       Transaction[] transactions,       string title,       string description,        string documentationUrl   );
Executing state renamed to Executable
StrategyFinalized event renamed to ProposalTimelocked
receiveStrategy function renamed to timelockProposal
Proposal states Uninitialized and Cancelled removed
state function renamed to proposalState for getting the state of a proposal
ProposalState Enum updated to:
enum ProposalState {       ACTIVE,       TIMELOCKED,       EXECUTABLE,       EXECUTED   }

UsulVetoGuard & VetoGuard

queueProposal function renamed to timelockProposal
ProposalQueued event renamed to ProposalTimelocked

Link to documentation
No response

@Da-Colon @tbwebb22 @mudrila @sethhrbek
The Usul updates are going to invalidate all of our current DAOs on the frontend, right?
Is there a way to support them and support migrating to the new contracts, or is that too much lift currently with everything else we're working on?

@tbwebb22 it might be good to update this ticket with further thoughts.
There was a method @mudrila pointed out that you refactored to return rather than revert, though I can't recall what that was?

Yeah so these contract updates will definitely invalidate the existing DAOs. I was planning on getting this issue updated at the end of this PR, since there are still a bunch of changes coming.
That function you're thinking of is the isPassed function on the LinearTokenVoting contract

yeah, definitely expecting that this will invalidate DAOs..
given that I'm now rethinking supporting Polygon for this event

@decent-dao/fractal not related to contract changes, but since we'll be making breaking changes, we might want to discuss changing the /nodes path for hierarchy, because we moved away from the term 'nodes' in the UI but that's still the path.


Add threaded websocket client

Adds a thread safe websocket client that can dispatch messages to multiple subscribed consumers through queues. Example usage here https://github.com/Momentous-Muses/code-jam/blob/fd7eb93abb625390e2342296e3ae22639d88e949/client/websocket_client/client.py#L115-L134
New message classes are created as Message subclasses, e.g.
@dataclass
class CustomMessage(websocket_client.Message):
    channel = "test_channel"
    type = "test_type"

    @classmethod
    def from_json_dict(cls, json_):
        return cls_created_from_json

    def to_json(self) -> str:
        return json.dumps(as_dict(self))

after the Message subclass is created, when a message with the given type is received on a channel, the class is created from the received json and sent to all queues on the given channel.
The uassage I have in mind is that the tkinter client will create the dispatcher, keep it around and for each queue it subscribes for (each channel will represent a different, separate part of the communication), it'll poll it with a function scheduled with app.after

There's merge conflicts for poetry.lock - i'm not entirely sure how this file works, does anyone else know how this conflict should be resolved?

Just waiting for the review from @ooliver1 before I can merge as he requested changes, once that's done I'm good to merge


Clarify Collections' interaction with WeakMap

Laravel Version
10.33
PHP Version
8.1.5
Database Driver & Version
N/A
Description
Laravel has collect() to produce a Collection; currently, the Collection accepts many types, one of which is PHP's Traversible.
It looks fine, but a problem: WeakMap implements Traversible. From the constructor definition/hinting, WeakMap instances are valid Collection "items". But as soon as such Collections are instantiated, a fatal error is always thrown:
TypeError: Illegal offset type.

Clearly, the underlying data structure of the Collection is an array, which obviously cannot accept objects as keys.
This is quite undocumented in the official Laravel docs in the Laravel website. However, the existing Collection constructor did not really check against WeakMaps.
I do know a bit about the covariance-contravariance thing that makes it a bit difficult to specifically reject WeakMap instances during typehinting.
A clarification is very much appreciated:

If Collections should not accept WeakMap, then perhaps the docs should be updated with this info; or, a more specific error message could be printed.
If Collections are supposed to accept WeakMap, then perhaps the Collection class needs some refactoring.

One very jarring experience with Laravel, is that e.g. I can do filter() on "real array" Collections, but I cannot do filter() on WeakMap instances, even when in native PHP, both can be traversed with something like
function filter(array|WeakMap $original, callable|Closure $predicate)
{
    $results = is_array($original) ? [] : new WeakMap();
    foreach ($original as $key => $value) {
        if ($predicate($value)) {
            $results[$key] = $value;
        }
    }
    return $results;
}

Steps To Reproduce
Using php artisan tinker [file]:
File contents:
<?php

$testMap = new WeakMap();
$testInstance = new stdClass();
$testInstance->haha = 'yes';
$testMap[$testInstance] = 3;

// should the following line be allowed or rejected?
$testCollection = collect($testMap);
// TypeError: Illegal offset type

// unreachable code: unhandled exception thrown
$testCollection->each(fn($x) => var_dump($x));

Submitted a PR to throw exception when trying to use $testCollection = collect($testMap);

Slow down! The problem is not that trivial.
I did some more research after submitting this, and realized WeakMap is not the only class affected by this clarification. For example, SplObjectStorage is also affected by this, because it instanceof Traversible, but obviously its keys are all objects, and then it cannot be sent into a Collection.
Basically, any class that is Traversible is a potential incompatible class of Collection. To throw more accurate exceptions, we will need to check whether the keys of Traversible is of the pseudotype array-key (basically int|string).
We will never know how many such incompatible classes out there that extends Traversible but have their keys in object type.
Therefore, the question remains:

refactor Collections to allow object keys?
improve detection of Traversible<object, mixed>?

I noticed this flag in iterator_to_array:

preserve_keys

Will setting it to false help with the situation? I see mentions of doing so to give out items inside SplObjectStorage instances.

You can't convert WeakMap to an array and we not planning to add support for WeakMap using Collection at the moment.


Export function rename for 1.0 release

**NOTE: Do not merge this PR until #1202 has been merged. I will need to update the Start-DbaMigration with this PR as it calls the Export-SqlLogin.
Changes proposed in this pull request:

Changed the few Export-Sql* functions to Export-Dba*
Updated help example for each command.
Will include update to Start-DbaMigration as it calls Export-DbaLogin (or will)
Updated the pester file name for Export-SqlUser.Test.ps1 to Export-DbaUser.Tests.ps1 (no code changes)

How to test this code:

[ ] Run Export-DbaLogin
[ ] Run Export-DbaUser
[ ] Run Export-DbaSpConfigure
[ ] Confirm test completes for Export-DbaUser.

I will update the psm1 and psd1 - thanks so much! @gbargsley would you be able to help with the website? Once they are updated, I can setup redirects.


add docker user group for systemd

Adding the docker group creation to the systemd dependencies

Added by the PR from Shambu. It contained other updates that were also necessary so I merged his over this.
But good catch finding this.


First example fails

Excited to try this but fails out of the gate. Bad sign. Get:
Debugger attached.
/Users/ted/src/orderscape/foodiebot/node_modules/typegoose/lib/prop.js:95
        throw new errors_1.NoMetadataError(key);
        ^

Error: There is no metadata for the "name" property.Check if emitDecoratorMetadata is enabled in tsconfig.jsonor check if you've declared a sub document's class after usage.

I've enabled emitDecoratorMetadata in tsconfig.js. using Mongoose 5.2.5, Node 9.2.1, Mongodb 3.6.6, Typescript 2.7.2.

sorry pilot error, wasn't fully compiled

Can you explain how you resolved it?
It doesn't work for me (event with full compilation...)
tsconfig
{     "compileOnSave": true,     "compilerOptions": {         "target": "es2017",         "module": "commonjs",         "noImplicitAny": false,         "skipLibCheck": true,         "experimentalDecorators": true,         "sourceMap": true,         "lib": [             "es2017",             "dom"         ]     },     "exclude": [         "node_modules",         "client"     ] }
package.json
{   "name": "test-typegoose",   "version": "1.0.0",   "description": "",   "main": "index.js",   "scripts": {     "test": "echo \"Error: no test specified\" && exit 1"   },   "author": "",   "license": "ISC",   "dependencies": {     "mongoose": "^5.4.11",     "typegoose": "^5.5.0"   } }

@HarelAshwal Sorry, it's been too long. Good luck, I hope you can resolve it.


wrapping fetch()

the fetch API, while a lot better than XMLHttpRequest is still ass, and sending a simple POST request requires a lot of annoying repetitive code
how do we feel about having get(endpoint, params) and post(endpoint, params) utility functions like i had in catbook-modern: https://github.com/mit6148-workshops/catbook-modern/blob/master/public/js/api.js

is okay to me, but we should document this file more heavily. eg what is the code in format params doing (students will have never seem Object.keys() and map/filter/reduce chaining). There is probably a slightly more verbose but more clear way of writing this.

i realize it also might be nice to add some cleaner error handling to this code
i remember last year, some people would send an invalid request to their server, and get some junk response that isn't JSON. this code would currently give some error message like "unexpected token in position 0when it tries to dores.json()` which is really cryptic

yes please LOL this would be super helpful. I think if this file is slightly more verbose and has better error handling then it's great to give to students.


api_docs: Format required or optional detail of param object field.

Formats and moves whether a field of an object in a request parameter is required or optional to be in the same location, and have the same formatting as, the general api parameter documentation.
Also, formats any examples within the object detailed description to be the same as the general api parameter documentation.
HTML api docs diff
Follow up to #20409 and #20899.
Screenshot of the subscribe endpoint parameter with examples added to show format change:

Nice, merged, thanks @laurynmm!


polarArea concentric line properties

Firstly, thank you to the community for contributing to this project.
I have search documentation vigorously and am unable to resolve an issue with applying properties to the concentric lines on a polarArea chart. Another developer (https://stackoverflow.com/q/55116860/9090980) seems to have the same request.
I would like to change the colors of the circles on a polarArea chart. The chart is presented on a page with a dark background and the lines are nearly lost. I have tried to work with the gridlines but the results did not work on the circles.
Cheers,

Its only available in master currently: https://www.chartjs.org/samples/master/scales/gridlines-style.html
https://codepen.io/kurkle/pen/oNNQyMZ

It's a bit more nuanced that that. Arrays of colours work in v2.9.3 but functions are only supported in master (v3 dev). https://codepen.io/etimberg/pen/oNNQQQP

Guys, thank you. You totally rock. This is very helpful. I updated my source to v2.9.3 and all is right in the Universe.


[MRG] cleanup variable names and logic related to "common" inputs

This PR is an attempt to clarify the code in terms of "external inputs" a.k.a. "external feeds". A lot of the diff is cosmetic, e.g., variable name changes. New tests and validations are also included.
One key point is renaming extinput to common. Our instinct was to go for rhythmic, as that's the only thing these are currently used for. However, the input mechanism is completely generic and could in principle support non-rhythmic inputs as well (e.g., Poisson bursts?).
The parameter handling of "common" and "unique" external feeds is still a little opaque in the code, but this should clarify. A future refactor/PR could remove the ExtFeed class (and parameter dicts) in favour of functions with signatures that clearly specify what parameters it expects: e.g., t_start, t_std, freq etc

@cjayb did you verify that the docs build alright and the examples don't change?

@cjayb if you address the last couple of comments, I'll merge the PR. Thanks a lot!

See if these help, there's surely plenty more for a future PR too ;)
I can confirm that the auto_examples in docs build (with some warnings)

@cjayb something is broken. I'm not sure if the alpha example was completely fixed in the previous PR or if it's a different issue.
I see this:

which is different from what's on the website:

Bugger... I'll have to investigate. It was still working before (a82f20f3a922a) I came in to fix the bug.

thank you! may the force be with you

@cjayb you inspired me to finish setting up the Circle CI. If you rebase against master, now you should see it building. This is what I got from the last build:
https://41-168215891-gh.circle-artifacts.com/0/html/auto_examples/index.html

Codecov Report

Merging #103 into master will increase coverage by 0.39%.
The diff coverage is 92.69%.

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
+ Coverage   88.86%   89.26%   +0.39%
==========================================
  Files          17       17
  Lines        1563     1593      +30
==========================================
+ Hits         1389     1422      +33
+ Misses        174      171       -3

Impacted Files
Coverage Δ

hnn_core/feed.py
65.98% <80.39%> (+3.39%)
:arrow_up:

hnn_core/network.py
83.69% <94.64%> (+0.64%)
:arrow_up:

hnn_core/basket.py
93.87% <100.00%> (ø)

hnn_core/cell.py
86.06% <100.00%> (ø)

hnn_core/params.py
84.93% <100.00%> (ø)

hnn_core/pyramidal.py
96.63% <100.00%> (ø)

hnn_core/tests/test_compare_hnn.py
100.00% <100.00%> (ø)

hnn_core/tests/test_feed.py
100.00% <100.00%> (ø)

hnn_core/tests/test_network.py
100.00% <100.00%> (ø)

hnn_core/tests/test_params.py
100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ec2ce1...89b3e10. Read the comment docs.

First of all, sorry about the git-mess: I managed to make a mess of my tree in the rebase...
I found the problem (fixed in
e26d98b), which was introduced into params.py with the bug I originally set out to fix (94fc21a).
But I also had to undo b2036e7 , which made test_compare_hnn fail due to mismatch with test_data/dpl.txt.
@rythorpe WDYT? I've basically rolled back what you set out to do in #98, and fixed the test you fixed by rolling back yet another commit! I'm sure I don't know whether anything makes sense anymore... Are we sure our gold standard is golden?
I think params.py:create_pext() and _feed_validate() are killing us. I'll open an issue to start discussing how to refactor...

yeah I figured the -2 was there for such reasons. TBH I was a little surprised that removing it allowed test_compare_hnn to pass... Can we conclude that all is OK then? Alpha seems to be working again, and the dipoles match.

I can confirm that the alpha simulation works now:
https://13-265240816-gh.circle-artifacts.com/0/html/auto_examples/plot_simulate_alpha.html#sphx-glr-auto-examples-plot-simulate-alpha-py
Let me check the rest of the code @cjayb !

regarding the random state, I think we should set the random state to be None by default (so they start at a truly random value) and then in the tests and examples, set them to match what HNN-GUI does. It's what we do in MNE and other packages. The random_state should not matter that much although it will certainly change the output. @rythorpe what do you think?

but let's reserve random state changes for another PR

@rythorpe is the PR good to go for you?

I wouldn't do that. People who run neuronal network simulations (which are pretty complex) want the same result each time and to have control over it.

From: Mainak Jas<EMAIL_ADDRESS>Sent: Tuesday, June 2, 2020 2:32 PM
To: jonescompneurolab/hnn-core<EMAIL_ADDRESS>Cc: Neymotin, Samuel (NKI<EMAIL_ADDRESS>Mention<EMAIL_ADDRESS>Subject: Re: [jonescompneurolab/hnn-core] [WIP] cleanup variable names and logic related to "common" inputs (#103)
ATTENTION: This email came from an external source. Do not open attachments or click on links from unknown senders or unexpected emails.
regarding the random state, I think we should set the random state to be None by default (so they start at a truly random value) and then in the tests and examples, set them to match what HNN-GUI does. It's what we do in MNE and other packages. The random_state should not matter that much although it will certainly change the output. @rythorpehttps://protect2.fireeye.com/v1/url?k=243670ed-7810474e-243489d8-000babda0106-78344c1851dea5ae&q=1&e=34364e87-6fa1-43a1-93a1-75a3870a57f0&u=https%3A%2F%2Fgithub.com%2Frythorpe what do you think?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://protect2.fireeye.com/v1/url?k=5c78c244-005ef5e7-5c7a3b71-000babda0106-1017722018c6f77e&q=1&e=34364e87-6fa1-43a1-93a1-75a3870a57f0&u=https%3A%2F%2Fgithub.com%2Fjonescompneurolab%2Fhnn-core%2Fpull%2F103%23issuecomment-637731453, or unsubscribehttps://protect2.fireeye.com/v1/url?k=708242cd-2ca4756e-7080bbf8-000babda0106-237b3eb022a11423&q=1&e=34364e87-6fa1-43a1-93a1-75a3870a57f0&u=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACS5H2BC6PI2TCEGW77MALLRUVAVTANCNFSM4NNIDFKA.

IMPORTANT NOTICE: This e-mail is meant only for the use of the intended recipient. It may contain confidential information which is legally privileged or otherwise protected by law. If you received this e-mail in error or from someone who was not authorized to send it to you, you are strictly prohibited from reviewing, using, disseminating, distributing or copying the e-mail. PLEASE NOTIFY US IMMEDIATELY OF THE ERROR BY RETURN E-MAIL AND DELETE THIS MESSAGE FROM YOUR SYSTEM. Thank you for your cooperation.

@rythorpe is the PR good to go for you?

No, see my comment above. I think we need to resolve the problem without undoing #98 since it affects the Network.gid_dict attribute.

I wouldn't do that. People who run neuronal network simulations (which are pretty complex) want the same result each time and to have control over it.

I see, I am not suggesting removing the option of setting random state. However, it does seem a bit odd to me that you would pick one random state over another for all users. This also makes me question how we can know for sure that the optimization routines were initialized correctly. cc @blakecaldwell . Let's reserve this for another issue or internal discussion!

@cjayb it sounds like the problem you encountered with the alpha example was caused by a seed discrepancy introduced by #98. Is that correct?

Nope, the problem was 1st that no common ('external') feeds were being included, ever. I thought it that simple, but the change to param.py was also causing a tripup: the alpha example is distal-only, where the proximal t0 is set to >'tstop'. The random seed is just a minor detail (doesn't explain the core problem this PR was trying to address).
I don't understand how your fix in #98 caused the alpha example to fail. Based on this discussion, I'd like to take a little longer to dig into what is going on. We might as well get this right... There's something really strange about proximal inputs affecting the alpha-example, even when all weights are zero! Hopefully I can sort the evoked example out as well.

I don't understand how your fix in #98 caused the alpha example to fail. Based on this discussion, I'd like to take a little longer to dig into what is going on. We might as well get this right... There's something really strange about proximal inputs affecting the alpha-example, even when all weights are zero! Hopefully I can sort the evoked example out as well.

This is certainly perplexing. I'll try to take a closer look at it as well. Thanks for working on this @cjayb. Your changes to the treatment of "common" inputs were much needed!

Hey guys, unless there is a trivial fix for the issue @rythorpe raised, I propose adding an XXX comment to the test saying what should pass and then addressing it in the next PR. I think that problem neatly fits into the issue of refactoring the ExtInput class and making everything consistent in that class.
This PR is pretty big already. It does improve things and fixes the alpha example, so I would call it progress.

Let me know when it's good to go @cjayb . Please set the PR status to [MRG] if you're ready.

The remaining great mystery is this: why does the alpha-example subtly fail when proximal common feeds are included in the gid_list, even though all (AMPA) weights are zero?
My money is on the way parreceive (and parconnect_from_src) is called for these inputs in the receiving pyramidal cells. Even though the weight is zero, the delay-value is not, which might lead to some wonky in the NetCon object. But that's beyond my current limits of understanding...

I think we need to resolve the problem without completely undoing parts of #98 since it affects the Network.gid_dict attribute.

@rythorpe I see your concern, but don't you think network.py:_get_src_type_and_pos() should gracefully handle some source types missing?
        # get type of cell and pos via gid
        src_type = self.gid_to_type(gid)
        type_pos_ind = gid - self.gid_dict[src_type][0]
        src_pos = self.pos_dict[src_type][type_pos_ind]

In any case, if dpl.txt is the gold standard, then our passing tests should mean we're OK for now, right?
@jasmainak assuming the last cosmetic commits don't break anything (!), I'm go for merge.

I'm now wondering how the parameter file handles the external inputs. Do you need to have all the parameters present for all the inputs? Are certain inputs turned off by making the weights = 0? Do you know @rythorpe ?
Yeah, let's merge this PR to keep the momentum and discuss more in a following PR.

It's in, thanks for the great contribution @cjayb !

@rythorpe I see your concern, but don't you think network.py:_get_src_type_and_pos() should gracefully handle some source types missing?
        # get type of cell and pos via gid
        src_type = self.gid_to_type(gid)
        type_pos_ind = gid - self.gid_dict[src_type][0]
        src_pos = self.pos_dict[src_type][type_pos_ind]

@cjayb yes I agree, but we just have to be consistent. As a user I'd like to see either all possible gids initialized in a network object or only the applicable ones.

Is the plan to just readdress the issue I encountered in #98 in a separate PR?
I thought the test you included (dpl.txt) was checking for the issue you were dealing with in #98. So because this PR also passes the (same) test, the issue should now be solved, no?

As for GID handling, I totally agree care should be taken in the next round of parameter handling cleanups!
@jasmainak currently the default way to ‘turn off’ an input is to set its start time to >stop time of the simulation... Only for some reason it didn’t seem to work for the proximal ‘common’ feeds!?

I thought the test you included (dpl.txt) was checking for the issue you were dealing with in #98. So because this PR also passes the (same) test, the issue should now be solved, no?

No. In #98 I implemented two tests: the first was in 'test_network.py' and validated whether or not all gids had been created. The values initialized in Network:p_common are used to assign Network:gid_dict and Network:_gid_list which are subsequently passed into Network:_create_all_spike_sources() where the ExtFeed objects are initialized. Thus, the stability of the gids passed into the ExtFeed constructor allows for stability of the seed assignment. (It's a mess, I know, but this is what caused the bug in #98.) The second test I implemented occurred, as you have noted, in 'test_compare_hnn.py` where we ensure that a simulation produces only the correct number and types of spike gids.
As the code is now, this PR can produce the buggy scenario addressed in #98 where shifting the start time of null rhythmic inputs (i.e., rhythmic inputs with zero synaptic weight) from >tstop to <tstop produces a different dipole:

As the code is now, this PR can produce the buggy scenario addressed in #98 where shifting the start time of null rhythmic inputs (i.e., rhythmic inputs with zero synaptic weight) from >tstop to <tstop produces a different dipole:

But this is how the rhythmic inputs work I think? There seems to be some inconsistency how the different kinds of inputs are handled. Some inputs are turned off by changing t0 > tstop and some are turned off using 0 weights?

Also I think we really need to separate out the methods into functions and document all the input parameters. Otherwise, it's really hard to get an overview of the different input types and what they do.

As the code is now, this PR can produce the buggy scenario addressed in #98 where shifting the start time of null rhythmic inputs (i.e., rhythmic inputs with zero synaptic weight) from >tstop to <tstop produces a different dipole:

But this is how the rhythmic inputs work I think? There seems to be some inconsistency how the different kinds of inputs are handled. Some inputs are turned off by changing t0 > tstop and some are turned off using 0 weights?

This is a very inconsistent and confusing way to handle turning on or off inputs. The whole point of #98 was to address this issue.

I agree, but let's clarify first what existed in master before #98, what was changed in the PR and what actually we want to achieve.
With respect to turning off the inputs, what happens with 0 weights, what happens when t0 > tstop, what happens to the gid_dict and what happens to the input spike array -- for different types of inputs? Can we summarize this somehow in a table or something?

With respect to turning off the inputs, what happens with 0 weights, what happens when t0 > tstop, what happens to the gid_dict and what happens to the input spike array -- for different types of inputs? Can we summarize this somehow in a table or something?

Here's my attempt to summarize it in a table. Hopefully this helps. I've attached a .pdf version for easy viewing and a zipped .odt file in case anyone wants to download and edit the 'desired' column of the table. Note that I used the default.json param file.
PR_98_summary_table.pdf
PR_98_summary_table.zip

@rythorpe some questions:

does #98 fix only seeds and gids or something else too?
what if the user provided seed is 0 and your gid is 0, how will you provide a negative seed?
it seems like a bug to me that the seed depends on gid. Do you know why this is the case?

again, if changing the seeds is going to have such a drastic effect on the waveforms, I think we should not set them by default. Otherwise, people will have false impressions about the reliability of their results and claims. Or am I missing something?

regarding 2nd point, see below:
In [6]: np.random.RandomState(-2)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-6-9809f15e3c8a> in <module>
----> 1 np.random.RandomState(-2)

mtrand.pyx in numpy.random.mtrand.RandomState.__init__()

mt19937.pyx in numpy.random.mt19937.MT19937._legacy_seeding()

mt19937.pyx in numpy.random.mt19937.MT19937._legacy_seeding()

ValueError: Seed must be between 0 and 2**32 - 1

regarding last point, see the numpy documentation. You only need to set one seed even to generate multiple random sequences. You just provide the right size and numpy will do the job for you.

* does #98 fix the issue of seeds and the range of values gids can take or something else too?

Yes, #98 stabilizes the gid assignment amidst changes to t0. This allows the user to 'turn off' a rhythmic input by setting its synaptic weights to zero without changing the seed (i.e., consistent with all the other input types).

* what if the user provided seed is 0 and your gid is 0, how will you provide a negative seed?

Given the order of gid assignment in Network and seed assignment in ExtFeed, the only way for this scenario to occur is if the network was initialized with 0 cells. Since such a scenario would render the whole simulation pointless (and most likely cause other errors to be thrown), I judged my somewhat janky solution of subtracting 2 from the seed value to be safe temporary solution. Also keep in mind that the only reason the '-2' was necessary was to make the seed match that of HNN. HNN contains the same buggy behavior where a user can load the 'default.param' parameter set, make a few modifications that might include setting tstop to longer (i.e., inadvertently causing t0<tstop for a null rhythmic input), and get an entirely different single-trial waveform from that shown in the tutorials.

* it seems like a bug to me that the seed depends on gid. Do you know why this is the case?

I'm not sure why it was set up this way, however, I think it's only a bug if the gid_dict fails to be the same every time. Using gids to set unique randomization seeds seems to be a reasonable approach, especially if a seed has to be assigned to each cell or unique input for parallelization where a random number generator may be required to run separately in each case. One alternative would be for the user to set one seed, and then use that seed to generate a random sequence of seeds to pass to each cell or input. This, however, would render a discrepancy with the current 'ground truth' dipole simulations we use in our tests.

again, if changing the seeds is going to have such a drastic effect on the waveforms, I think we should not set them by default. Otherwise, people will have false impressions about the reliability of their results and claims. Or am I missing something?

I disagree. First of all, the seed values we assign by default actually incorporate values the user has set or imported in their param file. Secondly, the only time someone really compares a single trial simulation to another is to do a sanity check where they are trying to replicate the exact dynamics of a previous simulation. This is what we do in our HNN comparison tests and a handful of tutorials. The tutorials, however, also emphasize that we are simulating a process that is only meaningful over many trials where the influence of any specific seed becomes negligible.

As the code is now, this PR can produce the buggy scenario addressed in #98 where shifting the start time of null rhythmic inputs (i.e., rhythmic inputs with zero synaptic weight) from >tstop to <tstop produces a different dipole:

I see. So the problem is in fact identical to that shown here as a subtle change in the alpha-example: somehow the all-zero-weight proximal 'common' input affects the simulation. I haven't been able to figure out why. But it's not a seed-issue, that's certain. Still think it's got to be in pyramidal.py:L5pyr().parreceive(), but no luck yet...

Using gids to set unique randomization seeds seems to be a reasonable approach, especially if a seed has to be assigned to each cell or unique input for parallelization where a random number generator may be required to run separately in each case.

Good point: the fact that several RNDs might be run on different CPUs means we have to assign gid-based seeds on initialisation.
I think the only reasonable way to approach to this is to figure out the underlying problem of common proximal inputs affecting the sim's when they shouldn't. Then write a fix that keeps the seed assignment consistent/predictable.

This, however, would render a discrepancy with the current 'ground truth' dipole simulations we use in our tests.

I think it might be reasonable to ask whether keeping the ground truth when several bugs have been found & fixed since it was created? It should be possible to re-create any tutorial waveform with a 'suitable' parameter set, of course.

I feel we might need a 3-way zoom/skype call to discuss some of the issues in person. Maybe I can discuss with @rythorpe first to understand what is the issue and then we can set a 3-way call if necessary!
Let me respond to a couple of things above:

But it's not a seed-issue, that's certain

humm, I'm not so certain. Changing the gid_dict changed the seeds so @rythorpe added -2 to account for it. Or do I misunderstand something?

I judged my somewhat janky solution of subtracting 2 from the seed value to be safe temporary solution.

okay this is fair enough

Secondly, the only time someone really compares a single trial simulation to another is to do a sanity check where they are trying to replicate the exact dynamics of a previous simulation

Isn't the optimization that is implemented in the GUI also doing seed-based single-trial simulations?

The tutorials, however, also emphasize that we are simulating a process that is only meaningful over many trials where the influence of any specific seed becomes negligible.

I know but I feel this is not enough. Most scientific software (cf MNE, scikit-learn) do not set the seed. The reason is that people will realize it's stochastic, then set the seed and do the experiment.

Good point: the fact that several RNDs might be run on different CPUs means we have to assign gid-based seeds on initialisation.

Not really, we can create the input vectors before sending things to parallel jobs.

One alternative would be for the user to set one seed, and then use that seed to generate a random sequence of seeds to pass to each cell or input.

This is exactly what numpy is already doing for you. We shouldn't reinvent the wheel.

I think it might be reasonable to ask whether keeping the ground truth when several bugs have been found & fixed since it was created?

I'm with you on that but I think this needs to be a broader discussion with @stephanie-r-jones . What would be a reasonable new ground truth? After fixing how many bugs do we consider that a new ground truth should be created?

I was just able to recreate the alpha discrepancy by modifying the seed.
In the attached .zip file I have include a modified version of the alpha example where I run 2 simulations for 710ms. Note that I change the seed core for the distal rhythmic input to 5 when previously it was 4. This was caused by the fact that I normalized the gid_dict attribute in #98 which cause an offset in the seed for a simulation when only a distal (and not proximal) input is set with t0<tstop. Here is an excerpt of my modified alpha example code as well as the output:
net = Network(params)
print('gid_dict =',net.gid_dict)
dpl = simulate_dipole(net)
dpl[0].plot()

# Increment seed for distal rhythmic input by 1
# Rerun simulatio
params['prng_seedcore_input_dist'] = 5
net = Network(params)
print('gid_dict =',net.gid_dict)
dpl = simulate_dipole(net)
dpl[0].plot()

Original:

With modified seed:

alpha_example.zip

Hi everyone, lots of progress! Great work! I've been following along and thought now would be a good time to jump in.

Secondly, the only time someone really compares a single trial simulation to another is to do a sanity check where they are trying to replicate the exact dynamics of a previous simulation

Isn't the optimization that is implemented in the GUI also doing seed-based single-trial simulations?

No, optimization is done with as many trials as you want. The tutorial uses 3:
https://jonescompneurolab.github.io/hnn-tutorials/optimization/optimization

I think it might be reasonable to ask whether keeping the ground truth when several bugs have been found & fixed since it was created?

I'm with you on that but I think this needs to be a broader discussion with @stephanie-r-jones . What would be a reasonable new ground truth? After fixing how many bugs do we consider that a new ground truth should be created?

We could say "consistent" instead of "ground truth". The ability to produce consistent results also means that we can explain any differences, whether a bug or a design choice. Right now this is difficult because hnn-core and HNN GUI have their own code for running simulations. However, the plan is to have HNN GUI run simulations with hnn-core in the near future.

@cjayb I had a video call with @rythorpe just a moment ago. To summarize the discussion:

Before #98, "common" inputs could be turned off by having t0 > tstop and in that case these inputs were not present in gid_dict
in #98 the "common" input was made to be always present in the gid_dict. This effectively means that the "common" inputs can no longer be turned off by having t0 > tstop. Since #98, the only way to turn them off is through 0 synaptic weights.
Unfortunately there were two issues with this pull request: 1) the "common" inputs were turned off (having an empty vector of input spikes) if any of the synaptic weights were 0 rather than "all" of them, and 2) the change in gid caused the seeds to change. The hack of gid - 2 did not actually fix the problem because you could have either 1 or 2 "common" inputs and this made the input spikes dependent on the number of "common" inputs
In #102 you fixed problem 1) but 2) was not fixed
The problem of 2) was discovered by me in #103
This was "fixed" in #103 by undoing "common" inputs to be always present (done in #98)

I proposed to @rythorpe to fix this by doing the following:

remove the hack of gid - 2
adjust the seeds in the example to account for the fact that the gid_dict always has "common" inputs now
fix the gid_test test to where it was before #103 so you check that "common" inputs are always present

Moving forward we would need to have a better way to set seeds but I think this can happen only after you make the pull request to refactor ExtFeed into separate functions. In the meanwhile @rythorpe can work on a pull request to fix these discrepancies. How does this sound like you? We can also do a video call if it helps you understanding the situation better.

Thanks for the update, LGTM! Glad it was the seed after all, really.
I’ll work on the extfeed next, leave sorting out the seeds vs gids to others. I’m still a little weirded out about having feeds in the net that aren’t actually doing anything... I don’t have an alternate suggestion though for fixing the train of seeds, so will leave it to others to decide.

the plan is to have HNN GUI run simulations with hnn-core in the near future.

Naming this project ‘core’ was a dead giveaway ;) ‘Consistent’ works for me. I think the design goal should be to make the code (and certainly the API) as clear as possible. Getting consistent results with an ‘old’ code base might get incrementally harder, but as long as we can do it that’s fine, of course.


build-rust-crate: disable incremental builds

Motivation for this change
The current implementation tries to disable incremental builds of Rust crates via -C incremental=no,
but according to rustc implementation1, -C incremental=no enables incremental builds with directory name no.
This patch removes the -C incremental argument to disable incremental builds.
I confirmed that running rustc by hand without -C incremental disables incremental builds, though I don't know how to test this in nixpkgs.
Things done

[ ] Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
Built on platform(s)

[ ] NixOS
[ ] macOS
[ ] other Linux distributions

[ ] Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
[ ] Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
[ ] Tested execution of all binary files (usually in ./result/bin/)
21.11 Release Notes (or backporting 21.05 Relase notes)

[ ] (Package updates) Added a release notes entry if the change is major or breaking
[ ] (Module updates) Added a release notes entry if the change is significant
[ ] (Module addition) Added a release notes entry if adding a new NixOS module

[ ] Fits CONTRIBUTING.md.

Result of nixpkgs-review pr 129763 at c3904019 run on x86_64-linux 1

4 packages built successfully:

cargo-download
carnix
systemfd
toml2nix

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:
https://discourse.nixos.org/t/how-to-look-at-the-build-directory-after-successful-build/14007/1

I confirmed that this patch does suppress incremental builds:
build log without this patch:
~$ cd /tmp

# compile a Rust crate using buildRustCrate without this patch
/tmp$ nix-shell ~/nixpkgs/default.nix -A cargo-download

# RUSTC_FORCE_INCREMENTAL=1 is needed for allowing rustc to do incremental builds
[nix-shell:/tmp]$ RUSTC_FORCE_INCREMENTAL=1 genericBuild
# this build log contains rustc invocation with `-C incremental=no`
<...build log...>

[nix-shell:/tmp/cargo-download-0.1.2.tar.gz]$ ls
# build result contains `no` directory
Cargo.toml  Cargo.toml.orig  LICENSE  README.md  no  src  target

[nix-shell:/tmp/cargo-download-0.1.2.tar.gz]$ ls no/
# with incremental builds information
cargo_download-cvuma7mz0ncd

build log with this patch:
# with this patch
~$ cd /tmp
[nix-shell:/tmp]$ sudo rm -r cargo-download-0.1.2.tar.gz

# we also allow incremental builds here
[nix-shell:/tmp]$ RUSTC_FORCE_INCREMENTAL=1 genericBuild
# but rustc invocation does not contain `-C incremental` this time
<build log>

[nix-shell:/tmp/cargo-download-0.1.2.tar.gz]$ ls
# therefore, no incremental builds directory here
Cargo.toml  Cargo.toml.orig  LICENSE  README.md  src  target

Haven't tested this but if our tests still pass that is fine for me.

Running tests via the following command succeeded (is this the right command?):
$ nix-build --expr 'let pkgs = import ./default.nix {}; in pkgs.callPackage ./pkgs/build-support/rust/build-rust-crate/test {}'


Updating GroundPrimitive doc with Apple support status

See #4375

Looks good. @pjcozzi should this doc fix close #4375?

@lilleyse yes, I'll close.
Thanks @rahwang!


Side content

Summary
Changes

Added Side Content
Reduce player Height

Screenshot

Ok @yokwejuste  thank you for these am on it

hi @yokwejuste
did you succeed to have the font type used?

@godsakani seems like you got some merge conflicts now!?

hi @yokwejuste did you succeed to have the font type used?

Not really, I'm trying to adapt for this, soon the PR will be completed


Configuring issue templates

Please create Issue templates so that we can organize what we have to do.
It would be good to organize PR template together.

About 'Bug reports' and 'Feature suggestions', it looks good to use the default issue templates provided by GitHub.
It looks good to add 'Question' and 'Help' additionally. For example,
Template name : Question
About :  Ask anything
Explain in detail what your question is about.
A clear and concise description of what the question is. Ex. I wonder why...
(It is good to use images.)
What do you think? Please give me feedback.

That sounds great! Please proceed like that. 👏👏

I added the issue templates !
Please check this out, and if there's anything wrong or that needs to be fixed, give me feedback.
Thank you :)

I checked! The template is detailed, so it'll be easy to leave an issue.
There seems to be nothing to fix. Thank you for your effort! 🙌

Really appreciate for your detailed feedback! :)

Same here ! You’ve done a thorough job!
It's great, seems that there's nothing to changed ;)

Thank you sujin :)


Update cisagov_N.yml

🗣 Description

Updated status of NI Products
fixed typo in 'notes'
removed duplicate entry

💭 Motivation and context

🧪 Testing

✅ Pre-approval checklist

[ ] This PR has an informative and human-readable title.
[ ] Changes are limited to a single goal - eschew scope creep!
[ ] All future TODOs are captured in issues, which are referenced
in code comments.
[ ] All relevant type-of-change labels have been added.
[ ] I have read the CONTRIBUTING document.
[ ] These code changes follow cisagov code standards.
[ ] All relevant repo and/or project documentation has been updated
to reflect the changes in this PR.
[ ] Tests have been added and/or modified to cover the changes in this PR.
[ ] All new and existing tests pass.

✅ Pre-merge checklist

[ ] Revert dependencies to default branches.
[ ] Finalize version.

✅ Post-merge checklist

[ ] Add a tag or create a release.

@Lcerkov - Makes sense.  I missed how 'Status' was computed and the fact that it is an 'OR' of the individual CVEs.  Was attempting to communicate that we have investigated against all of the CVEs, but only vulnerable to one.

@kyle-ni  Ahh, is it only vulnerable to CVE-2021-44228? If so, I will make that change and then merge the PR.

@Lcerkov Yes, that is correct.  Technically only CVE-2021-44228 applies for our products.

@kyle-ni Thanks for the clarification- I fixed that and merged the update!


Datasets used to train provided demo.pth model?

It would be great if the authors could discuss the training datasets used to train the demo.pth model for MORAN. Is it only the SynthText [Jaderberg et al., Gupta et al. [15]] that was used for training the model or other datasets were also used to fine-tune the model. I am particularly asking for the weights in file demo.pth

Yes, only the synthesized images were used for training.
M. Jaderberg, K. Simonyan, A. Vedaldi, and A. Zisserman. Synthetic data and artiﬁcial neural networks for natural scene text recognition. In Proceedings of Advances in Neural Information Processing Deep Learn. Workshop (NIPS-W), 2014.
A. Gupta, A. Vedaldi, and A. Zisserman. Synthetic data for text localisation in natural images. In Proceedings of Computer Vision and Pattern Recognition (CVPR), pages 2315–2324, 2016.
Please refer to https://github.com/Canjie-Luo/MORAN_v2#data-preparation.


Change the alignment and Text Size of elements on the footer

Hey @SauravMukherjee44 I would like to get this issue and I am confident I could resolve this pretty nicely.

Hello , I would like to work upon this, could you assign it to me


Non-daemon threads: Change in behaviour between v0.x & v1.0

In JPype1<1.0 user threads created in Java were being terminated early & abruptly if the main thread completed. Since JPype1 >= 1.0 user threads are now honoured.
This seems like a very welcome improvement :+1:, but I didn't see this in the changelog. Was this intentional, and if so, should it be documented?
Code to emulate (excuse my poor Java):
package org.hello_world;

public class hello_world {
    public static void thread_hello() throws Exception {
        System.out.println("Setting up a thread");

        Thread t = new Thread(() -> {
            int count = 0;
            while (count < 10) {
                count++;
                try {
                    Thread.currentThread().sleep(1000);
                } catch (InterruptedException e) {
                }
            }
            System.out.println("Thread finished");
        });
        t.setDaemon(false);
        t.start();

        Thread.sleep(750);
        System.out.println("Done in the main thread");
    }
}

Which, after compiling with:
javac ./org/hello_world/hello_world.java
jar -c ./org/hello_world/hello_world.java ./org/hello_world/hello_world.class > hello-world.jar

And running with:
import pathlib

import jpype as jp

HERE = pathlib.Path(__file__).parent.absolute()

def test():
    jp.startJVM(f'-Djava.class.path={HERE}/hello-world.jar', convertStrings=False)
    cls = jp.JClass("org.hello_world.hello_world")
    cls.thread_hello()

if __name__ == '__main__':
    test()

Can be compared with both JPype 0.7.5 and JPype 1.0.1:
$ python -m pip list | grep -i jpype
JPype1                0.7.5

$ time python run.py
<java class 'org.hello_world.hello_world'>
Setting up a thread
Done in the main thread

real	0m1.090s
user	0m0.395s
sys	0m0.045s

$ python -m pip list | grep -i jpype
JPype1                1.0.1

$ time python run.py
<java class 'org.hello_world.hello_world'>
Setting up a thread
Done in the main thread
Thread finished

real	0m10.428s
user	0m0.468s
sys	0m0.076s

Note: I found out about this through some convoluted static declaration in Java which was only hit in a few small cases in pytest tests. The symptom was that if I ran one of these special python tests (with the -k flag to pytest for instance), I was able to see that pytest ran everything as expected, even reporting the final results, but never actually terminated (so it just sat there without returning the prompt after running the tests). My first instinct was that this was another signal handler issue (e.g. #813) but couldn't find anything relevant in PyTest. As you can see, it turned out to be a complete red-herring, which I might not have fallen for with an relevant entry in the CHANGELOG. Hence the issue :wink:.

Hmm. It appears that you are correct.   It had a big change log entry at some point, but somehow when was doing the merge of the log it appears that the entire entry got drop.  (There was one pull request that totally scrambled the log and I thought I merged everything in).  Big whoops, my apologies.
Starting in 0.7.4 the shutdown sequence was reworked extensively.   The final trigger to enable the shutdown changes was pulled in 1.0.  The old system did not actually call the JVM shutdown, but actually just called halt.  The side effect of calling shutdown was honoring user threads, calling atshutdown hooks, flushing buffers, closing files properly, and unfortunately freezes on bad code.  You can still do a halt if necessary to get the old behavior.  I will have to dig back through the archives and find the missing entry and make sure it reappears in the change log.

Hi,
Thanks a lot for investigating this!
Unfortunately to me it looks like even daemon threads can block the JVM from exiting. Also, even calling sys.exit() explicitly does not terminate the JVM.
A mini example:
import jpype as jp
import sys

jp.startJVM()

Thread = jp.JClass("java.lang.Thread")
System = jp.JClass("java.lang.System")
tr = Thread(System.in_.read)
tr.setDaemon(True)
tr.start()

print(tr)
sys.exit(0)

Naively, I would expect the Java behavior:

Since tr is a daemon thread, I would not expect that it blocks the JVM from exiting.
I would expect that sys.exit(0) terminates the process, no matter what threads are still running.

If you need to force stop the process use Runtime.halt() I believe.
It should not prevent a shutdown on a daemon thread at least in the test cases I ran.   I will have to investigate that as a bug  but I am late getting the last patch release out, so I will have to do another later.

Thanks for the quick feedback! And sure, do the patch release first.
Yes, Runtime.halt() should work, but it is kind of a big hammer and potentially leaves a mess behind.
What looks a bit confusing to me is that there seems to be no difference between the main/python thread finishing (in which case I fully agree the process should survive if there are some active non daemon Java threads), and any thread calling exit() (which should ideally be consistent with System.exit() in Java - i.e. allow to run finalization, but terminate all threads, including non daemon ones).
The "java equivalent" of the above example has this behavior:
    public static void main(String[] args) throws Exception {
        Thread th = new Thread(() -> { try { System.in.read(); } catch (Exception e) { } });
        th.setDaemon(false);
        th.start();
        System.out.println("finishing main");
//        System.exit(0);
    }

Like this it will wait for the thread to finish; however if you un-comment System.exit(0), it will terminate.

The JNI docs seem to be fairly clear about the underlying jvm->DestroyJavaVM() call done in JPype for shutdown:

The JNI_DestroyJavaVM() function unloads a Java VM. As of JDK/JRE 1.1, only the main thread could unload the VM, by calling DestroyJavaVM. As of JDK/JRE 1.2, the restriction was removed, and any thread may call DestroyJavaVM to unload the VM.

The VM waits until the current thread is the only non-daemon user thread before it actually unloads. User threads include both Java threads and attached native threads. This restriction exists because a Java thread or attached native thread may be holding system resources, such as locks, windows, and so on. The VM cannot automatically free these resources. By restricting the current thread to be the only running thread when the VM is unloaded, the burden of releasing system resources held by arbitrary threads is on the programmer.

I think there are actually three issues here:
1: Changelog entry was missing. Fixed in #818. Thanks for that!
2: daemon threads should not prevent JVM shutdown (called via the jpype._JTerminate atexit handler). Code to reproduce (thanks Michi):
import jpype as jp

jp.startJVM()

Thread = jp.JClass("java.lang.Thread")
System = jp.JClass("java.lang.System")
tr = Thread(System.in_.read)
tr.setDaemon(True)
tr.start()

print("done - a daemon thread is running (awaiting stdin), but it shouldn't block immediate Python exit.")

In this case the atexit call to jpype._core.JTerminate is being called, but it doesn't result in termination of the program (i.e. I think this is a bug).
In contrast, Python does terminate with daemon threads (but not with non-daemon), as expected:
import sys, threading

def _thread():
    data = sys.stdin.readline()
    print("READ!", data)

thread = threading.Thread(target=_thread)
thread.daemon = True
thread.start()

print("Done! A daemon thread doesn't prevent Python exit")

3: non-daemon user threads should not block sys.exit called from Python (even if Runtime.halt() has to be called by JPype to do it). (interpreted from @michi42's statement "I would expect that sys.exit(0) terminates the process, no matter what threads are still running.")
import jpype as jp
import sys

jp.startJVM()

Thread = jp.JClass("java.lang.Thread")
System = jp.JClass("java.lang.System")
tr = Thread(System.in_.read)
tr.setDaemon(False)
tr.start()

print("done - a non-daemon user thread is running (awaiting stdin), which will prevent immediate exit. Calling sys.exit should do the right thing though...")

sys.exit(0)

What actually happens is that the main thread is exited, but the non-daemon thread continues running.
It turns out that Python also has the expectation that non-daemonic threads should be appropriately terminated by the programmer (as is the case with Java). For example:
import atexit
import sys
import threading
import time

sys_exit_called = False

def _thread():
    while True:
        print("." if not sys_exit_called else "x", end="", flush=True)
        time.sleep(1)

thread = threading.Thread(target=_thread, daemon=False)
thread.start()

atexit.register(print, 'atexit triggered')

print("Done! A non-daemon thread does exit the main thread, but the other running thread continues until it is done")

sys_exit_called = True
sys.exit()
print('Never reached')

Results in the following output:
.Done! A non-daemon thread does exit the main thread, but the other running thread continues until it is done
xxxx
^CException ignored in: <module 'threading' from '/media/important/github/jpype/jpype/env_jkd11/lib/python3.8/threading.py'>
Traceback (most recent call last):
  File "/media/important/github/jpype/jpype/env_jkd11/lib/python3.8/threading.py", line 1388, in _shutdown
    lock.acquire()
KeyboardInterrupt:
atexit triggered

So I'd conclude that the third issue is reasonable and consistent behaviour for both Java & Python, and isn't something that needs to be addressed (i.e. I don't think this is a bug).

I renamed the issue, since I think the only thing remaining is the second issue ("daemon threads should not prevent JVM shutdown"). I was tempted to create another issue for it, but all of the history is in the thread that I wanted to keep close to the issue.

I believe the title is incorrect.  I analyzed the test case and the issue is that an open JProxy call prevents shutdown.  As Python code has no way to get the exit call there is a reference counter in JProxy to make sure that we don't pull the rug out of on running Python code (which is a sure segmentation fault when it unwinds the stack).
In this case, the daemon thread creates a Python lambda when the System.in_.read is added.  That lambda is holding the ref count at 1.  But because it a daemon thread is allowed to keep running it wont be informed about the exit.   When the cleanup routine reaches wait for stragglers stage, it sees a ref count on the proxy is 1, and will be deadlocked on a waiting daemon thread.  It can't yank the plug because if the proxy exits after the C++ resource is gone it is a segfault.
About the only way I can prevent that would be for those proxies called from daemon threads to not count towards the open Proxy list.  But if that is the case then if something is frequently crossing the Python Java boundary on the daemon thread then we risk a segfault.   The problem here being that cleanup and atexit routine that are also alive at the exit time also need to be waited for as they are supposed be waited on.    If we cut off their resources then they will all fail too.
(comment out line 72, 95 from JPypeProxy to try the alternative)
So the best way I see for this is to not actually delete the C++ resources until after the JVM shutdown is complete so that we don't get a race condition between the atexit threads and the context resource cleanup.   Instead we would have to queue up all the resource destruction and wait for  shutdown to complete.  In the case of this exit it doesn't mater as all the memory is going to die anyway.  In the case of a Python called shutdown we can execute the actual clean up routines when post shutdown routine is reached.  This solution requires adding a global handler list which is only used once exit is started.  All freeing of resources are directed to list rather than executed immediately.  The JVM completes it shutdown with all Python resources still in place.  We then yank the plug.   But the problem here is that the original code would still crash as we still have a stack frame on the zombie thread that winds through the deleted C++ items to deal with.
Yuck.  Never a good solution.

Please test with #832 and see if it resolves the issues.


add openssf scorecard scanning/badge and clomonitor exemption

Please specify the area for this PR
What does does this PR do / why we need it:
This addresses the adding of certain open source best practices that we are adding to all of our repositories. These practices include the scorecard code scanning tool that checks for security vulnerabilities in our repositories as well as providing a score for the repository in the form of a badge. Additionally it adds the badge for the best practices checklist.
Note that on the CLOMonitor there will be missing checks for Artifact Hub and Recent Release. I don't see a point in marking Artifact Hub as exempt if there is items that can be added for this repository, and there are issues open to add them (helm charts). For Recent Release there was a release in 2022 but the check is only for <= 1 year. This will be remedied whenever a new release comes along and should not be exempted also.
Which issue(s) this PR fixes:
fixes https://github.com/devfile/api/issues/1387
PR acceptance criteria:

[ ] Test Coverage

Are your changes sufficiently tested, and are any applicable test cases added or updated to cover your changes?

Documentation (WIP)

[ ] Does the REST API doc need to be updated with your changes?
[ ] Does the registry library doc need to be updated with your changes?

How to test changes / Special notes to the reviewer:

For Recent Release there was a release in 2022 but the check is only for <= 1 year. This will be remedied whenever a new release comes along and should not be exempted also.

@Jdubrick FYI there is an open issue for the new release here: https://github.com/devfile/api/issues/1398. I've added a comment inside the issue to link this PR so the assignee will now that further actions might be required :)


Using frigate-hass-card with Frigate on different host.

I can't for the life of me get the frigate card to work with a Frigate install on a separate Linux host on my network. I only get either a black screen or the default screen.
How should this be configured if at all possible?
As I don't have a camera entity for the Frigate camera, I just chose a random camera and then configured the Frigate URL as follows:
type: custom:frigate-card
cameras:
  - camera_entity: camera.various_temp
    frigate:
      camera_name: Voor
      url: http://<IP_ADDRESS>:5000/
    live_provider: auto
view:
  default: clips
  camera_select: clips

I do have the Frigate proxy working in HASS and don't have the Frigate integration installed as it runs on another host.
help?

How should this be configured if at all possible?

This is absolutely possible (I do it myself).

I do have the Frigate proxy working in HASS and don't have the Frigate integration installed as it runs on another host.

If you expect Frigate functionality in the card, you must install the Frigate integration on the same HA instance on which the card is rendered. Lovelace cards can only talk to their local HA instance (and its integrations). So just configure the Frigate integration on your local HA, to point at your Frigate backend wherever that is.

I just installed the HASS integration to give this a try. After creating a basic config for frigate it started. Showing feed from the remote Frigate server doesn't work yet, I'm probably missing some required config.
Could you give me an example how to configure this? Do I need to specify my backend URL in frigate.yaml on HAOS, or is that all configured from the card config? I'd like to be able to view feeds, events and possibly recordings from the existing Frigate install on my other box.
Thanks!

Ok, this all works just fine. Closing this. Thanks for helping me understand!


Built in Booru

Is there any way you can get a built in booru? i would like to be able to download images and have the tags for them added to the booru is that alot to ask?

i meant so you can view the pictures in you downloaded in the downloader and have them retain their tags and everything
sorry i wasnt specific enough

kind of like this https://hydrusnetwork.github.io/hydrus/
but its built in

I'm not really sure about this.
I believe it should better be left to another software, such as picasa or a local Gelbooru. Browsing and downloading have many different features to implement (re-tagging, editing, resizing, more advanced search, indexing, etc) and I believe having it stored outside and browsed in a local booru (which is easily feasible, see here) be a better idea.
What I thought before was to allow to "search" such local booru the same way we can browse distant ones, but I don't think it would be wise to add a full featured booru directly inside the software (unless it's just something really basic, which is not what is asked here).

it dont need to be advanced, but i didnt know how advanced tags are

It would be quite an effort to use Grabber locally. It takes a little more know-how, but you can do your own local board, as already mentioned. More details are available in the wiki detailing 3 separate board types. I've really enjoyed using Shimmie in the form of a local board, as I feel is just a bit cleaner than Gelbooru and it also has a few different themes including one that mimics the look of Gelbooru 2.0.

I have a local board (shimmie) installed and tried adding it back into Grabber. Everything about it seemed to work which means if you're willing to go through the trouble of making your own board, importing the images into it as described in the docs, then all you have to do is add it as a source and you can use it directly in Grabber as was requested. Although for me, the images didn't show up. Probably a minor thing. The tags were all there, it just said no result. I will play around with the XML and see if I can get something to work with it.


lxd/instance/qemu: Only use KVM mode for feature detection on X86

This should fix a startup failure I've been getting on ARMv8 which I'm assuming is because our feature check logic doesn't set the GIC machine flag. It'd be neat if we could have the checkFeatures logic use the same way to spawn QEMU as we do for instances, but that's a bit too involved a change.

@tomponline good to go


POL-113 Azure Unused SQL Databases

Initial release
Description
Policy template checks for Azure SQL Databases that are unused by reviewing the DB connections and delete them after user approval.
Issues Resolved
This policy gets a list of Azure SQL Databases and uses the DB Connection metric to check for successful connections over a 30-day period. If there are no successful DB Connections the policy will terminate the SQL databases after the user approval.
Contribution Check List

[x] All tests pass.
[x] New functionality includes testing.
[x] New functionality has been documented in the README if applicable
[x] New functionality has been documented in CHANGELOG.MD

Template running link
https://governance.rightscale.com/org/1105/projects/60073/policy-incidents/5daff371a075f60089bf53a2
Screenshot

@nshivappa  Please check the conflicting file also.


4g get ip not working

Currently I am not able to get IP on 4g for ios, does anyone have this problem also?

I have the same problem...


Fixed not supported linq error when extracting terms sets running OnPremSP2013

Q
A

Bug fix?
yes

New feature?
no

New sample?
no

Related issues?

What's in this Pull Request?
Valid Scenario: On premises SharePoint 2013
Issue: Framework.Provisioning.ObjectHandlers.ObjectTermGroups:ExtractObjects() previously threw a method not supported exception when attempting to limit term sets to only those which where not the orphan term set. This fix replaces the inline linq query.

Hi @eric-scott-owens, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

thx Eric, we'll double check the change and get it merged.

Thanks Eric, your PR has been merged.


cca v0.7.0  'cca run (ios or android) --device' fails

Was having an issue deploying to ios device  so I upgraded (-g)   both cca and ios-deploy to their latest versions.
After doing  so I ran checkenv
cca v0.7.0
Android Development: SDK configured properly.
iOS Development: SDK configured properly.

all good.
I had an older version symlinked to a chrome app ( I like to develop on the desktop and then push to mobile when to test) so I ran cca create YourApp --link-to=path/to/manifest.json creating a new project in which to call cca run
Attempted Android
bjones$ cca run android --device
cca v0.7.0
## Running Cordova Command: platform add android
Creating Cordova project for the Android platform:
	Path: platforms/android
	Package: io.cordova.hellocordova
	Name: HelloCordova
	Activity: MainActivity
	Android target: android-22
Copying template files...
Android project created with<EMAIL_ADDRESS>/usr/local/lib/node_modules/cca/node_modules/q/q.js:141
                throw e;
                      ^
TypeError: undefined is not a function
    at Object.analyseManifest (/usr/local/lib/node_modules/cca/node_modules/cca-manifest-logic/src/analyse-manifest.js:97:31)
    at /usr/local/lib/node_modules/cca/src/pre-prepare.js:58:29
    at _fulfilled (/usr/local/lib/node_modules/cca/node_modules/q/q.js:820:54)
    at self.promiseDispatch.done (/usr/local/lib/node_modules/cca/node_modules/q/q.js:849:30)
    at Promise.promise.promiseDispatch (/usr/local/lib/node_modules/cca/node_modules/q/q.js:782:13)
    at /usr/local/lib/node_modules/cca/node_modules/q/q.js:590:44
    at runSingle (/usr/local/lib/node_modules/cca/node_modules/q/q.js:123:13)
    at flush (/usr/local/lib/node_modules/cca/node_modules/q/q.js:111:13)
    at process._tickCallback (node.js:355:11)

and ios
bjones$ cca run ios --device
cca v0.7.0
## Running Cordova Command: platform add ios
iOS project created with<EMAIL_ADDRESS>/usr/local/lib/node_modules/cca/node_modules/q/q.js:141
                throw e;
                      ^
TypeError: undefined is not a function
    at Object.analyseManifest (/usr/local/lib/node_modules/cca/node_modules/cca-manifest-logic/src/analyse-manifest.js:97:31)
    at /usr/local/lib/node_modules/cca/src/pre-prepare.js:58:29
    at _fulfilled (/usr/local/lib/node_modules/cca/node_modules/q/q.js:820:54)
    at self.promiseDispatch.done (/usr/local/lib/node_modules/cca/node_modules/q/q.js:849:30)
    at Promise.promise.promiseDispatch (/usr/local/lib/node_modules/cca/node_modules/q/q.js:782:13)
    at /usr/local/lib/node_modules/cca/node_modules/q/q.js:590:44
    at runSingle (/usr/local/lib/node_modules/cca/node_modules/q/q.js:123:13)
    at flush (/usr/local/lib/node_modules/cca/node_modules/q/q.js:111:13)
    at process._tickCallback (node.js:355:11)

Haven't seen this error before.

Here's line 97:
https://github.com/MobileChromeApps/mobile-chrome-apps/blob/master/cca-manifest-logic/src/analyse-manifest.js#L97
Could you double check that line 97 looks the same on your machine?
(/usr/local/lib/node_modules/cca/node_modules/cca-manifest-logic/src/analyse-manifest.js)
Perhaps you have a number as your manifest.mobile.json's "webview":  setting?

[Update]:   It does.   I determined the error of my ways.  I had permissions enabled for unsupported APIs.


CR: remove Layer from MapController constructor

I think, that GameController should not inherit cocos2d::Layer. It should just links all controllers with each other.
So. I suggest that each controller (for now lobby and map) create there own scene and layers, when they need.
Goal:

remove cocos::Layer from MapController constructor
MapController create it's own scene and pass it to cocos2d::Director by startGame event

This issue is solved by #53

ok let's close it then


Change Hackney100 to Work Placements

Coverage remained the same at 91.401% when pulling 5b7c2106f0b6f783207a760a1872745468d32693 on new-work-placements-section into 82806c106a90719a7b9bc9e3eef2588f98fdb555 on master.


Delay on swig command caused by version check

Would be nice to cache the version number rather than reach out to npm each time. This is more of an issue with tasks like transform-jsx which you run multiple times.

How about

Once a day check the global version is up to date
Every time check the local is up to date with the global

Thoughts?


Bug when compiling with v0.3.0: you need to release the pod version 0.3.1 with its updates.

The API v0.3.0 does not compile with Xcode Version 6.3.2 (6D2105).
For fixing that, you should release as quick as possible the pod version 0.3.1 including these two commits below:
https://github.com/thii/FontAwesome.swift/commit/4972f69f83b349528e080fef1dc34bb5bfcd05aa
https://github.com/thii/FontAwesome.swift/commit/7ba3a833beb11b593bb420d41fd1c2dd241d5f26
Note: thank you for keeping me updated of this future release.
Thank you in advance.

@King-Wizard Just released it.


num_workers support (Multi-Core CPU)

Can the document splitting and other CPU centric functions be made multi-core ?
When running
required_exts = [".csv"]
reader = SimpleDirectoryReader(...
documents = reader.load_data(show_progress=True,num_workers=6)
ragatouille_pack = RAGatouilleRetrieverPack(
documents, # List[Document]
llm=Ollama(model="phi", request_timeout=10000.0),
index_name="my_index",
top_k=5
)

This is a good idea, thanks for the suggestion!

@bclavie Shall I work on this?

@bclavie May I know if I can work on this? Kindly let me your thoughts on the implementation.


Slider too many digits after comma

The min-value of the slider is often up to 16 digits after the comma like
0.3125077572614787.
In Firefox this will cause a error like shown in the screenshot. Could you cap it to a specific amount of digits after the comma?
Chrome seems to work with it though.

This seems like an easy enough fix.  We're a little busy at the moment, but we'll get to it as soon as possible.  1.0.3 definitely!

This is fixed in 1.0.3 .. thanks for the catch.


Tool to add Meta To Existing Files

I guess the title says it all. With the addition of this feature, a sync tool is needed to make all existing files up to pace with the new ones.

Wouldn't you be able to do this exact thing simply by setting Grabber to the "save" option for dealing with files that already exist, then redownloading the file?

That will be an extreme waste of resources, not to mention a tremendous burden for the source websites. To do such a thing I will have to tune my download lists to the maximum, thousands of requests will get wasted to find the subset that I have.
Also, I guess you don't know how the APIs behave. The file does not get downloaded with its information, that is a separate operation. The tool will only get the tags and add them to the files, no need to download each file.


feat(nuxt-starter): rm @nuxt/composition-api

What This PR Does

swaps @nuxtjs/composition-api for @vue/composition-api
updates components to remove any nuxt-hooks from @nuxtjs/composition-api
moves logic in /plugins/globalSetup to /layouts/default

How To Test

git checkout LS-1417--rm-nuxt-composition
yarn dev
check that pages load and state is maintained: /search, /collections/clothing, /products/otto-shirt, /cart

Notes

seeing a warning in console on pages that fetch data Cannot stringify arbitrary non-POJOs NacelleClient - ex: /products/otto-shirt.  Working on sorting this out now

Looks really good! Little side bonus is that the current version of NuxtBridge requires us to move the globalSetup plugin to the layout anyways.

This route is no longer an option


Icons only configuration problem

If I change the icon=true config option then the buttons shrinks without showing anything. Image attached.

I'm having the same issue. I noticed that it's looking for "glyphicons-halflings-regular.woff2" instead of just ".woff". I tried changing the file extension to .woff2, but that did not work either.

@srinivasmangipudi Ok, I figure it out. You'll need to add the Font Awesome package for this to work.
meteor add fortawesome:fontawesome


examples/delete-engine.jl returns a 404

examples/delete-engine.jl returns a 404 when the wait loop successfully reaches the expected terminal state (when the call to get_engine return a 404) .. it should not report the error.

Note: This issue has been migrated to https://relationalai.atlassian.net/browse/RAI-4372.
This link is only accessible to employees of RelationalAI.


OCP on AWS overview layout update

Describe the bug
Labels and legends should appear similar to the OCP on AWS mock below:
https://redhat.invisionapp.com/share/HEO4ISHAT8Z#/screens/350233294
To Reproduce
Steps to reproduce the behavior:

Go to 'OCP on AWS overview'

Expected behavior
Just layout updates. Legend tooltips may be updated via a separate PR.
Screenshots

Closing in favor of new fit & finish issues.


Add noise data to timer tag operational tag

When summarizing operational time, use timer tag (575), any noise tags (975) and sentinel tag (550), not just timer tag, in case the timer tag was vandalized.

Do we really want to include observations from the noise tags? During the pilot season, I deliberately ignored noise when summarising operation times, because at times I found when noise was too bad the site actually couldn't read the timer tag (and presumably fish). The sentinel observations makes sense.

I updated summarise_timer_data so that is uses code endings 575 OR 550. I did not add noise 995s for the time being.
I suppose it comes down to the question of whether we want to summarise the time that the receivers were running OR the time that we believe receivers could detect fish. When I first did this summary I was really interested in "What proportion of the season do I believe the receivers could detect fish?" the really summary of performance.

K. I added the argument "include_noise" that includes "995" records when TRUE and excludes when FALSE. I also updated the roxygen documentation and updated vignette to reflect the change.


Secrets documentation has a broken link for how Secrets are automatically updated

https://github.com/kubernetes/website/blob/main/content/en/docs/concepts/configuration/secret.md has a section called "Editing a Secret". In that section there is a link to another section titled "#mounted-secrets-are-updated-automatically" but that section does not exist in the markdown.

There are two possible solutions I can think of, the first is to update the Secrets page to reference the section in the ConfigMaps page(https://kubernetes.io/docs/concepts/configuration/configmap/#mounted-configmaps-are-updated-automatically) the other is to recreate the section in the Secrets page that makes the Secrets page link valid again. The former seems the better option.

/assign

I also think that linking this to link would be the correct way of doing it.

There is already a issue #40042 (PR #40103) raised for the same.
/triage duplicate

/unassign

Duplicated by https://github.com/kubernetes/website/issues/40042
/close


Add ucontext struct for Linux/x86

I need the following structs from glibc for low level work. I pulled the information about the types from the glibc source. There is a lot of duplication due to the structure of rust libc. If the structs for other platforms would be added (and I might do that in the future), some of that repetiton might be eliminated.
I hid some structs in a module called 'detail'. I wanted to use the s!-macro for some structs that I did not want to provided public access to. Only the parts specified in the linux man page for getcontext are exposed publicly.

Thanks for the PR! Could this match the previous structure in terms of layout? (e.g. avoiding mod detail). You should be able to hide away private details for now with private fields I believe.

Ah yeah duplication is fine for now, we can always lift up later. Although it may be good to start defining ucontext_t on other platforms as well? Looks like OSX at least has the functions defined.

You should be able to hide away private details for now with private fields I believe.

I can and do that. However, some of these fields use structs as types, that I have to define. For simplicity, I wanted to use the s! macro to define them. That macro only allow to create public structs. If I put them in the exported module, we would export structs that no one could use.
It might be better to change s! to allow for non-public structs.

Yeah for now if you want to keep the structs private (is there a reason to intentionally do so?) then I'd recommend just doing _priv: [usize; amt]  and not worrying about the layout

Debugging through Travis takes a while. (There was no other reason for hiding the structs, than them not being documented in the man pages).

Thanks! I'm actually somewhat curious what it'll take to push this all the way over the finish line to a bunch of other platforms as well, so I'm gonna play around with a PR that does that for a bit.

Ah looks like these are so non-portable it's not worth it to try to add bindings for other platforms, so just a few stylistic nits and then this should be good to go.


Support multiple commands on multiple URLs in exechealthz

Fixes #1735.
Adding support for multiple commands on URLs for exec-healthz.
Example as below:
$ ./exechealthz --cmd="ls /tmp/test1" --url="/healthz1" --cmd="ls /tmp/test2" --url="/healthz2"

The --url flag indicates the path healthz server needs to serve on.
Notes: Number of cmds and URLs have to be the same (if more than 1). URL need to start with "/". URLs and cmds match up based on their orders (first URL to first cmd).
The previous behavior remains the same (as the previous examples in README).
@thockin @bprashanth @luxas

This change is

I don't know this sub-repo - any opportunity for tests?

Seems like the only way is to add unit tests under the same folder.
Is it Okay to run the main function in a go routine while using another go routine to verify?

no really.  I would try to test this by injecting a pkg/util/exec.Interface
which is a fake, and verifying that it gets called.  I don't want to derail
tooo much - it's all about confidence in the code.  You should not be stuck
doing tests for everything you touch, but if you CAN that is great.
On Tue, Sep 13, 2016 at 2:26 PM, Zihong Zheng<EMAIL_ADDRESS>wrote:

Seems like the only way is to add unit tests under the same folder.
Is it Okay to run the main function in a go routine while using another go
routine to verify?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kubernetes/contrib/pull/1739#issuecomment-246829977,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AFVgVNU1PrapupjoIWPsfSuh_UpusPFRks5qpxT1gaJpZM4J8Cev
.

Tests sound like a good idea. I didn't write them initially because I thought it was borderline complexity, but adding more probably pushes it over the edge.
You can give the execworker an interface to exec and a fakeClock, then trigger the clock and confirm the exec hits. Both those packages are available in main kuberepo (https://github.com/kubernetes/kubernetes/tree/master/pkg/util), but require godeps, which this project has thus far steered clear from. I leave it to you, maybe it's easier to add a godep, or just define your own fake.

@thockin @bprashanth Thanks for the detailed instructions. I tried exec and clock packages and added tests mainly for the execWorker.
Not sure to also test the web server or not. I consider holding a real server that occupy a port may introduce uncertainty and flaky result.
cc @matchstick

Sorry, there is a mistake in exechealthz_test.go. I misunderstood the usage of wait.Poll(). Will fix it soon.

Fixed.

recomputing cla status...

Now you need to tag this and push it.  I might also try to convince you to move this to github.com/thockin/go-build-template


Pytorch CPU Model Hyperparameter Training

Relates to #2216 feature request from github issues
Based on PR #2217
Relates to #2267 feature request from github issues

This PR updates the train_cpu_model-pytorch.py file as follows:

Introduces the ray tuning library for hyper parameter tuning.
Separates training, testing, and the training loop into separate functions as required by 1
Separation of the training set int o a training and validation set.
If a --leave-app-out is specified it is used as a test set after training

This approach to hyper parameter tuning follows the basic pytorch example here and the Ray Tune tutorial here
Hyper parameters tuned include:

Fully connected layer width
Fully connected layer depth
Learning rate
Batch Size
Loss Criterion
Different NN approaches (FF, LSTM, RNN)

Potential improvements:

Different search space selection for the config values (i.e. how we select learning rate, width, depth), potentially from here

Alternative to ASHA scheduler for early termination (AHB, PBT, RS, or BO)

epoch count tuning

This PR relies on the Ray Tune library being installed, as indicated in the README.md.  Ray has an Apache 2.0 license.
Example output during run:

Note that if the python file is edited during an active run the following error will occur:
Python type cannot be used as a value:
...
    def __init__(self, width_input, width_fc, depth_fc):
        super(FFNet, self).__init__()
              ~~~~~ <--- HERE
...

In order to address this issue when/if it occurs:

The final model is saved pre-torchscript conversion.  to -pre-torchscript
The best width, depth, batch size, and learning rate is provided at the end of hyperparameter tuning
A --convert-model option is added to rerun just the torchscript conversion portion.  This requires using --width and --depth:

--convert-model <input>-pre-torchscript --width <best_width> --depth <best_depth>

The --convert-model option currently limited to the Feed Forward Net.
Though Ray Tune may select LSTM the C++ CPUTorchAgent currently only supports basic Feed Forward Nets

An LSTM Net has been added. It appears to function reasonable well but has some limitations and may need further tuning and refinement.
The CPUTorchAgent does not support LSTM nets (it would require a change in input prepping) at this time, so this is primarily an example of how we may add additional models for selection in tuning.

This is great! I've started testing this as a drop-in for the hyperparameter sweep script that I was working on. I had one minor comment: I think this should work with RNNs without modification -- I think the torchscript model will handle maintaining the state for us. I'll test that and report back.


further pedantry

reading through more of the code and fixed some stuff (see commit messages)

Nice clean up. I really with travis would build. :+1:


Figure out unique IDs from CRSP

We should figure out if we can legally distribute those IDs.

At this point, a lot of the logic for this is in dlab-finance, but we need to have an architecture for reading in the CRSP data and matching it with the TAQ data as we stream it in. @davclark and @max-eddy should sit down together and work on this, but it'd be great for @max-eddy to take an initial stab. Recommendations:

[ ] Create a separate file in taq/ for reading / cleaning CRSP data
[ ] Add functionality that uses that file from raw_taq to add PERMNO to the TAQ data
[ ] Figure out how to make pandas a soft dependency. As it stands, we could have the above step involve a python dictionary.